Split the diff_errors logic from DicomValidator.validate_anonymised into a separate function

pixl_dcmd/src/pixl_dcmd/dicom_helpers.py

@@ -92,21 +92,17 @@ def validate_original(self, dataset: Dataset) -> ModuleErrors | None:
 
         return result.module_errors
 
-    def validate_anonymised(
-        self, dataset: Dataset, original_errors: ModuleErrors | None
-    ) -> dict[str, set[str]]:
-        """Check validation errors introduced during de-identification.
+    def validate_anonymised(self, dataset: Dataset) -> ModuleErrors:
+        """Validate an anonymised dataset.
 
         Args:
-            original_errors: module_errors returned by validate_original for
-                the dataset before anonymisation, or None if the dataset
-                hasn't been validated for pre-existing errors.
+            dataset: the anonymised dataset to validate.
 
         Returns:
-            new_errors: human-readable errors introduced by anonymisation,
-                keyed by module name. If original_errors is None, all errors
-                found after anonymisation are returned, as it's not possible
-                to tell which of them pre-existed.
+            module_errors: all validation errors found in the anonymised
+                dataset, keyed by module name then DICOM tag. Use
+                get_new_errors to determine which of these were introduced
+                by anonymisation.
 
         Raises:
             PixlSkipInstanceError: If dicom-validator could not validate the
@@ -119,8 +115,26 @@ def validate_anonymised(
                 f"dicom-validator returned status: {result.status}"
             )
             raise PixlSkipInstanceError(msg)
-        anon_errors = result.module_errors
+        return result.module_errors
+
+    def get_new_errors(
+        self, original_errors: ModuleErrors | None, anon_errors: ModuleErrors
+    ) -> dict[str, set[str]]:
+        """Compare validation errors before and after anonymisation.
 
+        Args:
+            original_errors: module_errors returned by validate_original for
+                the dataset before anonymisation, or None if the dataset
+                hasn't been validated for pre-existing errors.
+            anon_errors: module_errors returned by validate_anonymised for
+                the dataset after anonymisation.
+
+        Returns:
+            new_errors: human-readable errors introduced by anonymisation,
+                keyed by module name. If original_errors is None, all errors
+                found after anonymisation are returned, as it's not possible
+                to tell which of them pre-existed.
+        """
         if original_errors is None:
             logger.warning(
                 "Cannot determine whether validation errors were introduced by "

pixl_dcmd/src/pixl_dcmd/main.py

@@ -169,8 +169,8 @@ def anonymise_and_validate_dicom(
     anonymise_dicom(dataset, config=config)
 
     # Validate the anonymised dataset
-    validation_errors = dicom_validator.validate_anonymised(dataset, original_errors)
-    return validation_errors
+    anon_errors = dicom_validator.validate_anonymised(dataset)
+    return dicom_validator.get_new_errors(original_errors, anon_errors)
 
 
 def anonymise_dicom(

pixl_dcmd/tests/test_dicom_validator.py

@@ -31,7 +31,8 @@ def test_validation_check_works(vanilla_dicom_image_DX: Dataset) -> None:
     """
     validator = DicomValidator()
     original_errors = validator.validate_original(vanilla_dicom_image_DX)
-    assert not validator.validate_anonymised(vanilla_dicom_image_DX, original_errors)
+    anon_errors = validator.validate_anonymised(vanilla_dicom_image_DX)
+    assert not validator.get_new_errors(original_errors, anon_errors)
 
 
 def test_validation_after_anonymisation_works(
@@ -47,7 +48,8 @@ def test_validation_after_anonymisation_works(
     original_errors = validator.validate_original(vanilla_dicom_image_DX)
     anonymise_dicom(vanilla_dicom_image_DX, config=test_project_config)
 
-    assert not validator.validate_anonymised(vanilla_dicom_image_DX, original_errors)
+    anon_errors = validator.validate_anonymised(vanilla_dicom_image_DX)
+    assert not validator.get_new_errors(original_errors, anon_errors)
 
 
 @pytest.fixture()
@@ -65,7 +67,8 @@ def test_validation_passes_for_non_compliant_dicom(non_compliant_dicom_image) ->
     """
     validator = DicomValidator()
     original_errors = validator.validate_original(non_compliant_dicom_image)
-    assert not validator.validate_anonymised(non_compliant_dicom_image, original_errors)
+    anon_errors = validator.validate_anonymised(non_compliant_dicom_image)
+    assert not validator.get_new_errors(original_errors, anon_errors)
 
 
 def test_validation_fails_after_invalid_tag_modification(
@@ -79,9 +82,8 @@ def test_validation_fails_after_invalid_tag_modification(
     validator = DicomValidator()
     original_errors = validator.validate_original(vanilla_dicom_image_DX)
     del vanilla_dicom_image_DX.PatientName
-    validation_result = validator.validate_anonymised(
-        vanilla_dicom_image_DX, original_errors
-    )
+    anon_errors = validator.validate_anonymised(vanilla_dicom_image_DX)
+    validation_result = validator.get_new_errors(original_errors, anon_errors)
 
     assert len(validation_result) == 1
     assert "Patient" in validation_result.keys()
@@ -131,7 +133,8 @@ def test_validate_anonymised_returns_all_errors_when_original_unknown(
     validator = DicomValidator()
     del vanilla_dicom_image_DX.PatientName
 
-    validation_result = validator.validate_anonymised(vanilla_dicom_image_DX, None)
+    anon_errors = validator.validate_anonymised(vanilla_dicom_image_DX)
+    validation_result = validator.get_new_errors(None, anon_errors)
     assert "Patient" in validation_result.keys()
 
 
@@ -166,4 +169,4 @@ def test_validate_anonymised_raises_skip_instance_error_when_dataset_cannot_be_v
     validator = DicomValidator()
 
     with pytest.raises(PixlSkipInstanceError):
-        validator.validate_anonymised(dicom_missing_sop_class_uid, None)
+        validator.validate_anonymised(dicom_missing_sop_class_uid)