Ensure we validate against correct SPOClassUID in pixl_dcmd tests

pixl_dcmd/tests/conftest.py

@@ -126,67 +126,6 @@ def row_for_testing_image_with_pseudo_patient_id(
     return db_session
 
 
-def ids_for_parameterised_test(val: pathlib.Path) -> str:
-    """Generate test ID for parameterised tests"""
-    return str(val.stem)
-
-
-@pytest.fixture()
-@pytest.mark.parametrize(
-    ("yaml_file"), PROJECT_CONFIGS_DIR.glob("*.yaml"), ids=ids_for_parameterised_test
-)
-def row_for_dicom_testing(db_session, yaml_file) -> Session:
-    """
-    Insert a test row for the fake DICOM dataset generated by
-    pytest_pixl.dicom.generate_dicom_dataset.
-    """
-
-    config = load_project_config(yaml_file.stem)
-    modality = config.project.modalities[0]
-
-    extract = Extract(slug=config.project.name)
-    ds = pytest_pixl.dicom.generate_dicom_dataset(Modality=modality)
-    study_info = get_study_info(ds)
-
-    image_not_exported = Image(
-        mrn=study_info.mrn,
-        accession_number=study_info.accession_number,
-        study_uid=study_info.study_uid,
-        study_date=STUDY_DATE,
-        extract=extract,
-    )
-    with db_session:
-        db_session.add_all([extract, image_not_exported])
-        db_session.commit()
-
-    return db_session
-
-
-@pytest.fixture()
-def row_for_single_dicom_testing(db_session) -> Session:
-    """
-    Insert a test row for the fake DICOM dataset generated by
-    pytest_pixl.dicom.generate_dicom_dataset.
-    """
-
-    extract = Extract(slug=TEST_PROJECT_SLUG)
-    ds = pytest_pixl.dicom.generate_dicom_dataset()
-    study_info = get_study_info(ds)
-
-    image_not_exported = Image(
-        mrn=study_info.mrn,
-        accession_number=study_info.accession_number,
-        study_uid=study_info.study_uid,
-        study_date=STUDY_DATE,
-        extract=extract,
-    )
-    with db_session:
-        db_session.add_all([extract, image_not_exported])
-        db_session.commit()
-
-    return db_session
-
-
 @pytest.fixture()
 def directory_of_mri_dicoms() -> Generator[pathlib.Path, None, None]:
     """Directory containing MRI DICOMs suitable for testing."""
@@ -279,41 +218,15 @@ def mock_get(key, default) -> Optional[str]:
 
 
 @pytest.fixture()
-def vanilla_dicom_image_DX(row_for_dicom_testing) -> Dataset:
-    """
-    A DICOM image with diffusion data to test the anonymisation process.
-    Private tags were added to match the tag operations defined in the project config, so we can
-    test whether the anonymisation process works as expected when defining overrides.
-    The row_for_mri_dicom_testing dependency is to make sure the database is populated with the
-    project slug, which is used to anonymise the DICOM image.
-    """
-    return generate_dicom_dataset(Modality="DX")
-
-
-@pytest.fixture()
-def vanilla_single_dicom_image_DX(row_for_single_dicom_testing) -> Dataset:
+def vanilla_dicom_image_DX() -> Dataset:
     """
     A DICOM image with diffusion data to test the anonymisation process.
     Private tags were added to match the tag operations defined in the project config, so we can
     test whether the anonymisation process works as expected when defining overrides.
-    The row_for_single_dicom_testing dependency is to make sure the database is populated with the
-    project slug, which is used to anonymise the DICOM image.
     """
     return generate_dicom_dataset(Modality="DX")
 
 
-@pytest.fixture()
-def vanilla_dicom_image_MR(row_for_dicom_testing) -> Dataset:
-    """
-    A DICOM image with MX data to test the anonymisation process.
-    Private tags were added to match the tag operations defined in the project config, so we can
-    test whether the anonymisation process works as expected when defining overrides.
-    The row_for_mri_dicom_testing dependency is to make sure the database is populated with the
-    project slug, which is used to anonymise the DICOM image.
-    """
-    return generate_dicom_dataset(Modality="MR")
-
-
 @pytest.fixture(scope="module")
 def test_project_config() -> PixlConfig:
     return load_project_config(TEST_PROJECT_SLUG)

pixl_dcmd/tests/test_dicom_validator.py

@@ -20,19 +20,19 @@
 from pydicom import Dataset
 
 
-def test_validation_check_works(vanilla_single_dicom_image_DX: Dataset) -> None:
+def test_validation_check_works(vanilla_dicom_image_DX: Dataset) -> None:
     """
     GIVEN a DICOM dataset
     WHEN the dataset is validated against itself (withouth anonymisation)
     THEN no errors should be raised
     """
     validator = DicomValidator()
-    validator.validate_original(vanilla_single_dicom_image_DX)
-    assert not validator.validate_anonymised(vanilla_single_dicom_image_DX)
+    validator.validate_original(vanilla_dicom_image_DX)
+    assert not validator.validate_anonymised(vanilla_dicom_image_DX)
 
 
 def test_validation_after_anonymisation_works(
-    vanilla_single_dicom_image_DX: Dataset,
+    vanilla_dicom_image_DX: Dataset,
     test_project_config,
 ) -> None:
     """
@@ -41,17 +41,17 @@ def test_validation_after_anonymisation_works(
     THEN no errors should be raised
     """
     validator = DicomValidator()
-    validator.validate_original(vanilla_single_dicom_image_DX)
-    anonymise_dicom(vanilla_single_dicom_image_DX, config=test_project_config)
+    validator.validate_original(vanilla_dicom_image_DX)
+    anonymise_dicom(vanilla_dicom_image_DX, config=test_project_config)
 
-    assert not validator.validate_anonymised(vanilla_single_dicom_image_DX)
+    assert not validator.validate_anonymised(vanilla_dicom_image_DX)
 
 
 @pytest.fixture()
-def non_compliant_dicom_image(vanilla_single_dicom_image_DX: Dataset) -> Dataset:
+def non_compliant_dicom_image(vanilla_dicom_image_DX: Dataset) -> Dataset:
     """A DICOM dataset that is not compliant with the DICOM standard."""
-    del vanilla_single_dicom_image_DX.PatientName
-    return vanilla_single_dicom_image_DX
+    del vanilla_dicom_image_DX.PatientName
+    return vanilla_dicom_image_DX
 
 
 def test_validation_passes_for_non_compliant_dicom(non_compliant_dicom_image) -> None:
@@ -66,17 +66,17 @@ def test_validation_passes_for_non_compliant_dicom(non_compliant_dicom_image) ->
 
 
 def test_validation_fails_after_invalid_tag_modification(
-    vanilla_single_dicom_image_DX,
+    vanilla_dicom_image_DX,
 ) -> None:
     """
     GIVEN a DICOM dataset
     WHEN an invalid tag operation is performed (e.g. deleting a required tag)
     THEN validation should return a non-empty list of errors
     """
     validator = DicomValidator()
-    validator.validate_original(vanilla_single_dicom_image_DX)
-    del vanilla_single_dicom_image_DX.PatientName
-    validation_result = validator.validate_anonymised(vanilla_single_dicom_image_DX)
+    validator.validate_original(vanilla_dicom_image_DX)
+    del vanilla_dicom_image_DX.PatientName
+    validation_result = validator.validate_anonymised(vanilla_dicom_image_DX)
 
     assert len(validation_result) == 1
     assert "Patient" in validation_result.keys()

pixl_dcmd/tests/test_main.py

@@ -24,6 +24,7 @@
 import pydicom
 import pytest
 import sqlalchemy
+from pytest_check import check
 from core.db.models import Image
 from core.dicom_tags import (
     PrivateDicomTag,
@@ -45,7 +46,6 @@
 )
 from pytest_pixl.dicom import generate_dicom_dataset
 from pytest_pixl.helpers import run_subprocess
-from conftest import ids_for_parameterised_test
 
 if typing.TYPE_CHECKING:
     from core.project_config.pixl_config_model import PixlConfig
@@ -71,7 +71,7 @@ def _get_mri_diffusion_tags(
     so we can test whether the manufacturer overrides work during anonymisation
     """
     tag_ops = load_tag_operations(config)
-    mri_diffusion_overrides = tag_ops.manufacturer_overrides[0]
+    mri_diffusion_overrides = tag_ops.manufacturer_overrides[1]
 
     manufacturer_overrides = [
         override
@@ -115,26 +115,26 @@ def test_enforce_allowlist_removes_overlay_plane() -> None:
 
 
 def test_anonymisation(
-    vanilla_single_dicom_image_DX: pydicom.Dataset,
+    vanilla_dicom_image_DX: pydicom.Dataset,
     test_project_config: PixlConfig,
 ) -> None:
     """
     Test whether anonymisation works as expected on a vanilla DICOM dataset
     """
 
-    orig_patient_id = vanilla_single_dicom_image_DX.PatientID
-    orig_patient_name = vanilla_single_dicom_image_DX.PatientName
-    orig_study_date = vanilla_single_dicom_image_DX.StudyDate
+    orig_patient_id = vanilla_dicom_image_DX.PatientID
+    orig_patient_name = vanilla_dicom_image_DX.PatientName
+    orig_study_date = vanilla_dicom_image_DX.StudyDate
 
-    anonymise_dicom(vanilla_single_dicom_image_DX, config=test_project_config)
+    anonymise_dicom(vanilla_dicom_image_DX, config=test_project_config)
 
-    assert vanilla_single_dicom_image_DX.PatientID != orig_patient_id
-    assert vanilla_single_dicom_image_DX.PatientName != orig_patient_name
-    assert vanilla_single_dicom_image_DX.StudyDate != orig_study_date
+    assert vanilla_dicom_image_DX.PatientID != orig_patient_id
+    assert vanilla_dicom_image_DX.PatientName != orig_patient_name
+    assert vanilla_dicom_image_DX.StudyDate != orig_study_date
 
 
 def test_anonymise_unimplemented_tag(
-    vanilla_single_dicom_image_DX: pydicom.Dataset,
+    vanilla_dicom_image_DX: pydicom.Dataset,
     test_project_config: PixlConfig,
 ) -> None:
     """
@@ -150,16 +150,14 @@ def test_anonymise_unimplemented_tag(
     nested_block.add_new(0x0011, "OB", b"")
 
     # create private sequence tag with the nested dataset
-    block = vanilla_single_dicom_image_DX.private_block(
-        0x0013, "VR OB CREATOR", create=True
-    )
+    block = vanilla_dicom_image_DX.private_block(0x0013, "VR OB CREATOR", create=True)
     block.add_new(0x0010, "SQ", [nested_ds])
 
-    anonymise_dicom(vanilla_single_dicom_image_DX, config=test_project_config)
+    anonymise_dicom(vanilla_dicom_image_DX, config=test_project_config)
 
-    assert (0x0013, 0x0010) in vanilla_single_dicom_image_DX
-    assert (0x0013, 0x1010) in vanilla_single_dicom_image_DX
-    sequence = vanilla_single_dicom_image_DX[(0x0013, 0x1010)]
+    assert (0x0013, 0x0010) in vanilla_dicom_image_DX
+    assert (0x0013, 0x1010) in vanilla_dicom_image_DX
+    sequence = vanilla_dicom_image_DX[(0x0013, 0x1010)]
     assert (0x0013, 0x1011) not in sequence[0]
 
 
@@ -190,6 +188,11 @@ def test_anonymise_and_validate_as_external_user(
     assert dataset != pydicom.dcmread(dataset_path)
 
 
+def ids_for_parameterised_test(val: pathlib.Path) -> str:
+    """Generate test ID for parameterised tests"""
+    return str(val.stem)
+
+
 @pytest.mark.parametrize(
     ("yaml_file"), PROJECT_CONFIGS_DIR.glob("*.yaml"), ids=ids_for_parameterised_test
 )
@@ -200,22 +203,21 @@ def test_anonymise_and_validate_dicom(caplog, request, yaml_file) -> None:
     WHEN the anonymisation and validation process is run
     THEN the dataset should be anonymised and validated without any warnings or errors
     """
-    caplog.clear()
     caplog.set_level(logging.WARNING)
     config = load_project_config(yaml_file.stem)
-    modality = config.project.modalities[0]
-    dicom_image = request.getfixturevalue(f"vanilla_dicom_image_{modality}")
-
-    validation_errors = anonymise_and_validate_dicom(
-        dicom_image,
-        config=config,
-    )
-
-    assert "WARNING" not in [record.levelname for record in caplog.records]
-    assert not validation_errors
+    for modality in config.project.modalities:
+        caplog.clear()
+        dicom_image = generate_dicom_dataset(Modality=modality)
+        validation_errors = anonymise_and_validate_dicom(
+            dicom_image,
+            config=config,
+        )
+        with check:
+            assert "WARNING" not in [record.levelname for record in caplog.records]
+            assert not validation_errors
 
 
-@pytest.mark.usefixtures("row_for_single_dicom_testing")
+@pytest.mark.usefixtures()
 def test_anonymisation_with_overrides(
     mri_diffusion_dicom_image: pydicom.Dataset,
     test_project_config: PixlConfig,
@@ -417,7 +419,9 @@ def test_should_exclude_series(dicom_series_to_exclude, dicom_series_to_keep):
 
 
 def test_can_nifti_convert_post_anonymisation(
-    row_for_single_dicom_testing, tmp_path, directory_of_mri_dicoms, tag_scheme
+    tmp_path,
+    directory_of_mri_dicoms,
+    tag_scheme,
 ):
     """Can a DICOM image that has passed through our tag processing be converted to NIFTI"""
     # Create a directory to store anonymised DICOM files

pixl_dcmd/tests/test_tag_schemes.py

@@ -16,6 +16,7 @@
 from pathlib import Path
 
 import pytest
+import pytest_check
 from core.project_config import load_project_config
 from core.project_config.tag_operations import TagOperations, load_tag_operations
 from decouple import config
@@ -39,7 +40,7 @@ def test_merge_base_only_tags(base_only_tag_scheme):
     THEN the result should be the same as the base file
     """
     tags = merge_tag_schemes(base_only_tag_scheme)
-    expected = [*base_only_tag_scheme.base[0], *base_only_tag_scheme.base[1]]
+    expected = [tag for base in base_only_tag_scheme.base for tag in base]
     count_tags = dict()
     for tag in expected:
         key = f"{tag['group']:04x},{tag['element']:04x}"
@@ -49,9 +50,9 @@ def test_merge_base_only_tags(base_only_tag_scheme):
             count_tags[key] = 1
 
     for key, values in count_tags.items():
-        assert (
-            values == 1
-        ), f"{key} is replicated please check config files to remove it"
+        pytest_check.equal(
+            values, 1, msg=f"{key} is replicated please check config files to remove it"
+        )
 
     assert tags == expected
 

projects/configs/tag-operations/base.yaml

@@ -95,6 +95,14 @@
   group: 0x0008
   element: 0x9205
   op: "keep"
+- name: "Volumetric Properties"
+  group: 0x0008
+  element: 0x9206
+  op: "keep"
+- name: "Volume Based Calculation Technique"
+  group: 0x0008
+  element: 0x9207
+  op: "keep"
 #################################### 0010 Group ###################################
 #
 #
@@ -136,7 +144,19 @@
   group: 0x0018
   element: 0x1020
   op: "keep"
+- name: "Field Of View Dimension"
+  group: 0x0018
+  element: 0x1149
+  op: "keep"
 #CT, MR, PET, US, X-Ray, and RT Images
+- name: "Imager Pixel Spacing"
+  group: 0x0018
+  element: 0x1164
+  op: "keep"
+- name: "Grid"
+  group: 0x0018
+  element: 0x1166
+  op: "keep"
 - name: "Focal Spot"
   group: 0x0018
   element: 0x1190