Merge pull request #785 from yarikoptic/fix/xa_pulse_seq_name

Refactor create_seqinfo tiny bit to avoid duplication and add logging; and in tests to reuse list of dicom paths

heudiconv/dicoms.py

@@ -94,15 +94,19 @@ def create_seqinfo(
     series_desc = get_typed_attr(dcminfo, "SeriesDescription", str, "")
     protocol_name = get_typed_attr(dcminfo, "ProtocolName", str, "")
 
-    if dcminfo.get([0x18, 0x24]):
-        # GE and Philips
-        sequence_name = dcminfo[0x18, 0x24].value
-    elif dcminfo.get([0x19, 0x109C]):
-        # Siemens
-        sequence_name = dcminfo[0x19, 0x109C].value
-    elif dcminfo.get([0x18, 0x9005]):
-        # Siemens XA
-        sequence_name = dcminfo[0x18, 0x9005].value
+    for k, m in (
+        ([0x18, 0x24], "GE and Philips"),
+        ([0x19, 0x109C], "Siemens"),
+        ([0x18, 0x9005], "Siemens XA"),
+    ):
+        if v := dcminfo.get(k):
+            sequence_name = v.value
+            lgr.debug(
+                "Identified sequence name as %s coming from the %r family of MR scanners",
+                sequence_name,
+                m,
+            )
+            break
     else:
         sequence_name = ""
 

heudiconv/tests/test_dicoms.py

@@ -21,7 +21,7 @@
     parse_private_csa_header,
 )
 
-from .utils import TESTS_DATA_PATH
+from .utils import TEST_DICOM_PATHS, TESTS_DATA_PATH
 
 # Public: Private DICOM tags
 DICOM_FIELDS_TO_TEST = {"ProtocolName": "tProtocolName"}
@@ -180,26 +180,17 @@ def test_get_datetime_from_dcm_wo_dt() -> None:
     assert get_datetime_from_dcm(XA30_enhanced_dcm) is None
 
 
-dicom_test_data = [
-    (dw.wrapper_from_file(d_file), [d_file], op.basename(d_file))
-    for d_file in glob(op.join(TESTS_DATA_PATH, "*.dcm"))
-]
-
-
-@pytest.mark.parametrize("mw,series_files,series_id", dicom_test_data)
+@pytest.mark.parametrize("dcmfile", TEST_DICOM_PATHS)
 def test_create_seqinfo(
-    mw: dw.Wrapper,
-    series_files: list[str],
-    series_id: str,
+    dcmfile: str,
 ) -> None:
-    seqinfo = create_seqinfo(mw, series_files, series_id)
-    assert seqinfo.sequence_name != ""
-    pass
-
+    mw = dw.wrapper_from_file(dcmfile)
+    seqinfo = create_seqinfo(mw, [dcmfile], op.basename(dcmfile))
+    assert seqinfo.sequence_name
 
-def test_get_reproducible_int() -> None:
-    dcmfile = op.join(TESTS_DATA_PATH, "phantom.dcm")
 
+@pytest.mark.parametrize("dcmfile", TEST_DICOM_PATHS)
+def test_get_reproducible_int(dcmfile: str) -> None:
     assert type(get_reproducible_int([dcmfile])) is int
 
 

heudiconv/tests/utils.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+from glob import glob
 import logging
 import os.path as op
 from pathlib import Path
@@ -9,6 +10,14 @@
 
 HEURISTICS_PATH = op.join(heudiconv.heuristics.__path__[0])
 TESTS_DATA_PATH = op.join(op.dirname(__file__), "data")
+# Do relative to curdir to shorten in a typical application,
+# and side-effect test that tests do not change curdir.
+TEST_DICOM_PATHS = [
+    op.relpath(x)
+    for x in glob(op.join(TESTS_DATA_PATH, "**/*.dcm"), recursive=True)
+    # exclude PhoenixDocuments
+    if "PhoenixDocument" not in x
+]
 
 lgr = logging.getLogger(__name__)