Merge pull request #45 from brown-bnc/scan_contains_dicom

Add scan_contains_dicom

pyproject.toml

@@ -17,6 +17,7 @@ pytest = "^5.3.5"
 python-dotenv = "^0.12.0"
 black = {version = "^19.10b0", allow-prereleases = true}
 flake8 = "^3.8.3"
+responses = "^0.12.0"
 
 [tool.poetry.scripts]
 bids-postprocess ="xnat_tools.bids_postprocess:run"

tests/unit/test_bids_utils.py

@@ -1,12 +1,15 @@
 import os
-import shutil
 import pytest
+import requests
+import responses
+import shutil
 import xnat_tools.bids_utils as utils
 
 from xnat_tools.bids_utils import (
-    handle_scanner_exceptions,
-    bidsmap_scans,
     bidsify_dicom_headers,
+    bidsmap_scans,
+    handle_scanner_exceptions,
+    scan_contains_dicom,
 )
 
 
@@ -124,9 +127,9 @@ def test_bidsify_dicom_headers_with_protocol_name_mismatch(mocker):
     series_description_mock = mocker.Mock(value="quux")
 
     side_effect = [
-        protocol_name_mock,      # 1st call to check if the ProtocolName
-        protocol_name_mock,      # 2nd call to set the ProtocolName
-        series_description_mock, # 3rd call to set SeriesDescription
+        protocol_name_mock,  # 1st call to check if the ProtocolName
+        protocol_name_mock,  # 2nd call to set the ProtocolName
+        series_description_mock,  # 3rd call to set SeriesDescription
     ]
 
     dataset = mocker.MagicMock()
@@ -149,3 +152,90 @@ def test_bidsify_dicom_headers_with_protocol_name_mismatch(mocker):
 
     assert protocol_name_mock.value == series_description
     assert series_description_mock.value == series_description
+
+
+@responses.activate
+def test_scan_contains_dicom_no_dicom():
+    """Test scan_contains_dicom without any DICOM resources"""
+    host = "https://example.com/xnat"
+    session = "SESSION-01"
+    scanid = "SCAN-01"
+
+    url = f"{host}/data/experiments/{session}/scans/{scanid}/resources"
+    payload = {"ResultSet": {"Result": [{"file_count": "10", "label": "NOTDICOM",}],}}
+
+    responses.add(responses.GET, url, json=payload, status=200)
+    connection = requests.Session()
+
+    assert scan_contains_dicom(connection, host, session, scanid) == False
+
+
+@responses.activate
+def test_scan_contains_dicom_many_dicom():
+    """Test scan_contains_dicom with more than one DICOM resource"""
+    host = "https://example.com/xnat"
+    session = "SESSION-01"
+    scanid = "SCAN-01"
+
+    url = f"{host}/data/experiments/{session}/scans/{scanid}/resources"
+    payload = {
+        "ResultSet": {
+            "Result": [
+                {"file_count": "10", "label": "DICOM",},
+                {"file_count": "20", "label": "DICOM",},
+            ],
+        }
+    }
+
+    responses.add(responses.GET, url, json=payload, status=200)
+    connection = requests.Session()
+
+    assert scan_contains_dicom(connection, host, session, scanid) == False
+
+
+@responses.activate
+def test_scan_contains_dicom_empty_file_count():
+    """Test scan_contains_dicom without file_count field"""
+    host = "https://example.com/xnat"
+    session = "SESSION-01"
+    scanid = "SCAN-01"
+
+    url = f"{host}/data/experiments/{session}/scans/{scanid}/resources"
+    payload = {"ResultSet": {"Result": [{"label": "DICOM",}],}}
+
+    responses.add(responses.GET, url, json=payload, status=200)
+    connection = requests.Session()
+
+    assert scan_contains_dicom(connection, host, session, scanid) == True
+
+
+@responses.activate
+def test_scan_contains_dicom_zero_file_count():
+    """Test scan_contains_dicom with file_count equal to zero"""
+    host = "https://example.com/xnat"
+    session = "SESSION-01"
+    scanid = "SCAN-01"
+
+    url = f"{host}/data/experiments/{session}/scans/{scanid}/resources"
+    payload = {"ResultSet": {"Result": [{"file_count": "0", "label": "DICOM",}],}}
+
+    responses.add(responses.GET, url, json=payload, status=200)
+    connection = requests.Session()
+
+    assert scan_contains_dicom(connection, host, session, scanid) == False
+
+
+@responses.activate
+def test_scan_contains_dicom_many_file_count():
+    """Test scan_contains_dicom with file_count greater than 1"""
+    host = "https://example.com/xnat"
+    session = "SESSION-01"
+    scanid = "SCAN-01"
+
+    url = f"{host}/data/experiments/{session}/scans/{scanid}/resources"
+    payload = {"ResultSet": {"Result": [{"file_count": "10", "label": "DICOM",}],}}
+
+    responses.add(responses.GET, url, json=payload, status=200)
+    connection = requests.Session()
+
+    assert scan_contains_dicom(connection, host, session, scanid) == True

xnat_tools/bids_utils.py

@@ -203,6 +203,48 @@ def bidsify_dicom_headers(filename, series_description):
         dataset.data_element("SeriesDescription").value = series_description
 
 
+def scan_contains_dicom(connection, host, session, scanid):
+    """Checks to see if the scan has suitable DICOM files for BIDS conversion"""
+    resp = get(
+        connection,
+        host + "/data/experiments/%s/scans/%s/resources" % (session, scanid),
+        params={"format": "json"},
+    )
+
+    dicomResourceList = [
+        r for r in resp.json()["ResultSet"]["Result"] if r["label"] == "DICOM"
+    ]
+
+    # NOTE (BNR): A scan contains multiple resources. A resource can be thought
+    #             of as a folder. We only want a single DICOM folder. If we have
+    #             multiple, something is weird. If we don't have any DICOM
+    #             resources the scan doesn't have any DICOM images. We only
+    #             download the scan if there's a single DICOM resource
+    if len(dicomResourceList) <= 0:
+        return False
+    elif len(dicomResourceList) > 1:
+        return False
+    else:
+        dicomResource = dicomResourceList[0]
+
+    # NOTE (BNR): We only want to process the scan if we have dicom files. But
+    #       sometimes the file_count field is empty and we process anyway even
+    #       though that might make things break later
+    if dicomResource.get("file_count") is None:
+        _logger.warning(
+            'DICOM resources for scan %s have a blank "file_count". '
+            "I cannot check to see if there are no files. "
+            "I am not skipping the scan. "
+            "This may lead to errors later if there are no DICOM files in the scan.",
+            scanid,
+        )
+        return True
+    elif int(dicomResource["file_count"]) == 0:
+        return False
+
+    return True
+
+
 def assign_bids_name(
     connection, host, subject, session, scans, build_dir, bids_session_dir,
 ):
@@ -214,43 +256,9 @@ def assign_bids_name(
     """
 
     for scanid, seriesdesc in scans:
-        _logger.info("---------------------------------")
-
-        # Get scan resources
-        r = get(
-            connection,
-            host + "/data/experiments/%s/scans/%s/resources" % (session, scanid),
-            params={"format": "json"},
-        )
-        scanResources = r.json()["ResultSet"]["Result"]
-        _logger.debug(
-            "Found resources %s." % ", ".join(res["label"] for res in scanResources)
-        )
-
-        dicomResourceList = [res for res in scanResources if res["label"] == "DICOM"]
-
-        if len(dicomResourceList) == 0:
-            _logger.debug("Scan %s has no DICOM resource." % scanid)
-            continue
-        elif len(dicomResourceList) > 1:
-            _logger.debug("Scan %s has more than one DICOM resource Skipping." % scanid)
+        if not scan_contains_dicom(connection, host, session, scanid):
             continue
 
-        dicomResource = dicomResourceList[0] if len(dicomResourceList) > 0 else None
-
-        usingDicom = True if (len(dicomResourceList) == 1) else False
-
-        if dicomResource is not None and dicomResource["file_count"]:
-            if int(dicomResource["file_count"]) == 0:
-                _logger.info(
-                    "DICOM resource for scan %s has no files. Skipping." % scanid
-                )
-                continue
-        else:
-            _logger.info(
-                'DICOM resources for scan %s have a blank "file_count", so I cannot check to see if there are no files. I am not skipping the scan, but this may lead to errors later if there are no files.'
-                % scanid
-            )
 
         # BIDS sourcedatadirectory for this scan
         _logger.info(f"bids_session_dir: {bids_session_dir}")
@@ -262,15 +270,13 @@ def assign_bids_name(
             os.mkdir(bids_scan_directory)
         else:
             _logger.warning(
-                f"{bids_scan_directory} already exists. See documentation to understad how xnat_tools handles repeaded sequences."
+                f"{bids_scan_directory} already exists. See documentation to understad how xnat_tools handles repeated sequences."
             )
 
-        # Get DICOMs
-        if usingDicom:
-            filesURL = host + "/data/experiments/%s/scans/%s/resources/DICOM/files" % (
-                session,
-                scanid,
-            )
+        filesURL = host + "/data/experiments/%s/scans/%s/resources/DICOM/files" % (
+            session,
+            scanid,
+        )
 
         r = get(connection, filesURL, params={"format": "json"})
         # Build a dict keyed off file name