Add more tests for _should_exclude_series

SAFEHR-data · Jan 27, 2025 · 6b707cb · 6b707cb
1 parent b6872b9
commit 6b707cb
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 33 deletions.
diff --git a/pixl_core/src/core/project_config/pixl_config_model.py b/pixl_core/src/core/project_config/pixl_config_model.py
@@ -180,7 +180,7 @@ def is_series_number_excluded(self, manufacturer: str, series_number: str | None
             return True
 
         exclude_series_numbers = self._get_manufacturer(manufacturer).exclude_series_numbers
-        return any(series_number.find(filt) != -1 for filt in exclude_series_numbers)
+        return any(str(series_number).find(filt) != -1 for filt in exclude_series_numbers)
 
     def is_manufacturer_allowed(self, manufacturer: str) -> bool:
         """

diff --git a/pixl_dcmd/tests/test_main.py b/pixl_dcmd/tests/test_main.py
@@ -407,40 +407,39 @@ def test_no_pseudo_patient_id_processing(
     )
 
 
-@pytest.fixture()
-def dicom_series_to_keep() -> list[pydicom.Dataset]:
-    series = [
-        "",
-        "whatever",
-    ]
-    return [_make_dicom(s) for s in series]
-
-
-@pytest.fixture()
-def dicom_series_to_exclude() -> list[pydicom.Dataset]:
-    series = [
-        "positioning",
-        "foo_barpositioning",
-        "positioningla",
-        "scout",
-        "localiser",
-        "localizer",
-        # Matching should be case insensitive
-        "lOcALIsER",
-    ]
-    return [_make_dicom(s) for s in series]
-
-
-def _make_dicom(series_description) -> pydicom.Dataset:
-    return generate_dicom_dataset(SeriesDescription=series_description)
+def _make_dicom(series_description, manufacturer, series_number) -> pydicom.Dataset:
+    return generate_dicom_dataset(
+        SeriesDescription=series_description,
+        Manufacturer=manufacturer,
+        SeriesNumber=series_number,
+    )
 
 
-def test_should_exclude_series(dicom_series_to_exclude, dicom_series_to_keep):
+@pytest.mark.parametrize(
+    ("series_description", "manufacturer", "series_number", "expect_exclude"),
+    [
+        ("", "Company", "1", False),
+        ("whatever", "Company", "1", False),
+        ("whatever", "Company", None, True),
+        ("positioning", "Company", "1", True),
+        ("foo_barpositioning", "Company", "1", True),
+        ("positioningla", "Company", "1", True),
+        ("scout", "Company", "1", True),
+        ("localiser", "Company", "1", True),
+        ("localizer", "Company", "1", True),
+        ("lOcALIsER", "Company", "1", True),
+        ("", "DifferentCompany", "1", True),
+        ("", "Company", "1234567890", True),
+    ],
+)
+def test_should_exclude_series(
+    series_description, manufacturer, series_number, expect_exclude
+):
     config = load_project_config(TEST_PROJECT_SLUG)
-    for s in dicom_series_to_keep:
-        assert not _should_exclude_series(s, config)
-    for s in dicom_series_to_exclude:
-        assert _should_exclude_series(s, config)
+    ds = _make_dicom(series_description, manufacturer, series_number)
+    series_number = ds.get("SeriesNumber")
+    print(f"{series_number=}", type(series_number), str(series_number))
+    assert _should_exclude_series(ds, config) == expect_exclude
 
 
 def test_can_nifti_convert_post_anonymisation(

diff --git a/projects/configs/test-extract-uclh-omop-cdm.yaml b/projects/configs/test-extract-uclh-omop-cdm.yaml
@@ -28,7 +28,7 @@ allowed_manufacturers:
   - regex: "^carestream"
     exclude_series_numbers: []
   - regex: "^company"
-    exclude_series_numbers: []
+    exclude_series_numbers: ["1234567890"]
   - regex: "^philips"
     exclude_series_numbers: []