From 99cce0dca8e8c9b438091713489cd59315797302 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 16 Sep 2026 10:15:25 +0100 Subject: [PATCH 1/2] Split dicom_validator.validate_anonymised into two functions Added a new method DicomValidator.get_new_errors --- pixl_dcmd/src/pixl_dcmd/dicom_helpers.py | 38 ++++++++++++++++-------- pixl_dcmd/src/pixl_dcmd/main.py | 4 +-- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py b/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py index b9a61216e..657718bd2 100644 --- a/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py +++ b/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 " diff --git a/pixl_dcmd/src/pixl_dcmd/main.py b/pixl_dcmd/src/pixl_dcmd/main.py index 3c5f76377..04352dff9 100644 --- a/pixl_dcmd/src/pixl_dcmd/main.py +++ b/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( From da38785a744b0061f9b288dbbf93d48d9e0e39eb Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 16 Sep 2026 10:15:28 +0100 Subject: [PATCH 2/2] Update dcmd tests for get_new_errors split --- pixl_dcmd/tests/test_dicom_validator.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pixl_dcmd/tests/test_dicom_validator.py b/pixl_dcmd/tests/test_dicom_validator.py index 121ad9471..f77986adc 100644 --- a/pixl_dcmd/tests/test_dicom_validator.py +++ b/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)