From 797336e200dd8b0d2897f7e5fab720dcaeefe272 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 16 Sep 2026 10:14:29 +0100 Subject: [PATCH 1/5] Update dicom-validator to 0.83 Update uv.lock --- docker/orthanc/Dockerfile | 2 +- pixl_dcmd/pyproject.toml | 2 +- uv.lock | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docker/orthanc/Dockerfile b/docker/orthanc/Dockerfile index e54f00b14..bd373d04d 100644 --- a/docker/orthanc/Dockerfile +++ b/docker/orthanc/Dockerfile @@ -32,7 +32,7 @@ FROM pixl_orthanc_uv AS pixl_orthanc_with_spec # Do it in dead end build stage to discard this environment afterwards, # and because the spec is only needed in orthanc-anon. RUN uv venv -RUN uv pip install dicom-validator==0.7.3 +RUN uv pip install dicom-validator==0.8.3 COPY ./orthanc/orthanc-anon/plugin/download_dicom_spec.py /etc/orthanc/download_dicom_spec.py RUN --mount=type=cache,target=/root/.cache,id=dlspec \ python3 /etc/orthanc/download_dicom_spec.py diff --git a/pixl_dcmd/pyproject.toml b/pixl_dcmd/pyproject.toml index 17f8cf8ef..0dc90dd37 100644 --- a/pixl_dcmd/pyproject.toml +++ b/pixl_dcmd/pyproject.toml @@ -11,7 +11,7 @@ dependencies = [ "arrow==1.4.0", "deid==0.4.12", "dicom-anonymizer==2.0.0", - "dicom-validator==0.7.3", + "dicom-validator==0.8.3", "logger==1.4", "pydicom==3.0.2", "pydicom-data", diff --git a/uv.lock b/uv.lock index cdaba1933..74840dfc5 100644 --- a/uv.lock +++ b/uv.lock @@ -735,16 +735,16 @@ wheels = [ [[package]] name = "dicom-validator" -version = "0.7.3" +version = "0.8.3" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "lxml" }, { name = "pydicom" }, { name = "pyparsing" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/a8/db/922c5d3c662aff47e1ab1a64fe9203b5ea813ea493b7b216c03b9bfb4676/dicom_validator-0.7.3.tar.gz", hash = "sha256:4ee8376688fb94ca33c48af34bc5a93dd3e3743abefdc1d16b70eba41c4aa542", size = 63526, upload-time = "2025-10-13T17:32:49.564Z" } +sdist = { url = "https://files.pythonhosted.org/packages/dd/d7/7ae7a25d260613118f6d63545a2a0454aa209d06600c319ed9455a2d2887/dicom_validator-0.8.3.tar.gz", hash = "sha256:7b92f809f892ad37954e7b3c8c12bf10512496d6ae9a36db2dab3c1da456260e", size = 77652, upload-time = "2026-08-28T16:15:17.711Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/6e/8d/b3401e93c7301e96315d4d816f5643815f5e6797a8c511e56e91e237b546/dicom_validator-0.7.3-py3-none-any.whl", hash = "sha256:2bffcdf2ae774ef18c521774561987098bfe8c157fdd1fa5bf04f552a195499f", size = 72200, upload-time = "2025-10-13T17:32:48.204Z" }, + { url = "https://files.pythonhosted.org/packages/e1/17/c4fc7e4cf99a8c9217bc1a3d40f2996f3ddece11d22ce27e9bd220b29342/dicom_validator-0.8.3-py3-none-any.whl", hash = "sha256:755e800f05df8357b38902debbdb0588dd9206a281828fc788d80aa79a993927", size = 89443, upload-time = "2026-08-28T16:15:16.354Z" }, ] [[package]] @@ -2150,7 +2150,7 @@ requires-dist = [ { name = "core", extras = ["test"], marker = "extra == 'test'", editable = "pixl_core" }, { name = "deid", specifier = "==0.4.12" }, { name = "dicom-anonymizer", specifier = "==2.0.0" }, - { name = "dicom-validator", specifier = "==0.7.3" }, + { name = "dicom-validator", specifier = "==0.8.3" }, { name = "logger", specifier = "==1.4" }, { name = "pydicom", specifier = "==3.0.2" }, { name = "pydicom-data" }, From 45d224d665c8c6dfed29ccd27bf93769e46e61ab Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 16 Sep 2026 10:14:34 +0100 Subject: [PATCH 2/5] Use EditionReader.get_edition_path and EditionReader.dicom_info_for_edition get_edition_path will download the spec if needed dicom_info_for_edition loads the spec from disk --- orthanc/orthanc-anon/plugin/download_dicom_spec.py | 4 +--- pixl_dcmd/src/pixl_dcmd/dicom_helpers.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/orthanc/orthanc-anon/plugin/download_dicom_spec.py b/orthanc/orthanc-anon/plugin/download_dicom_spec.py index 04e4f3f4d..99fcee6ba 100644 --- a/orthanc/orthanc-anon/plugin/download_dicom_spec.py +++ b/orthanc/orthanc-anon/plugin/download_dicom_spec.py @@ -20,6 +20,4 @@ edition = "2024e" download_path = str(Path.home() / "dicom-validator") edition_reader = EditionReader(download_path) -destination = edition_reader.get_revision(edition, recreate_json=False) -json_path = Path(destination, "json") -EditionReader.load_dicom_info(json_path) +edition_reader.get_edition_path(edition) diff --git a/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py b/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py index e7bea3ed7..f5f8b7c70 100644 --- a/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py +++ b/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py @@ -44,9 +44,7 @@ def __init__(self, edition: str = "current"): standard_path = str(Path.home() / "dicom-validator") with _redirect_stdout_to_debug(logger): edition_reader = EditionReader(standard_path) - destination = edition_reader.get_revision(self.edition, False) - json_path = Path(destination, "json") - self.dicom_info = EditionReader.load_dicom_info(json_path) + self.dicom_info = edition_reader.dicom_info_for_edition(self.edition) def validate_original(self, dataset: Dataset) -> dict | None: """Check pre-existing validation errors in a dataset. From 248c4aafaf3633f0b8833b8ad37cf08137bd52f9 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 16 Sep 2026 10:14:48 +0100 Subject: [PATCH 3/5] Update dicom-validator to 0.9.0 and update uv.lock --- docker/orthanc/Dockerfile | 2 +- pixl_dcmd/pyproject.toml | 2 +- uv.lock | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docker/orthanc/Dockerfile b/docker/orthanc/Dockerfile index bd373d04d..0ade50478 100644 --- a/docker/orthanc/Dockerfile +++ b/docker/orthanc/Dockerfile @@ -32,7 +32,7 @@ FROM pixl_orthanc_uv AS pixl_orthanc_with_spec # Do it in dead end build stage to discard this environment afterwards, # and because the spec is only needed in orthanc-anon. RUN uv venv -RUN uv pip install dicom-validator==0.8.3 +RUN uv pip install dicom-validator==0.9.0 COPY ./orthanc/orthanc-anon/plugin/download_dicom_spec.py /etc/orthanc/download_dicom_spec.py RUN --mount=type=cache,target=/root/.cache,id=dlspec \ python3 /etc/orthanc/download_dicom_spec.py diff --git a/pixl_dcmd/pyproject.toml b/pixl_dcmd/pyproject.toml index 0dc90dd37..f14a03855 100644 --- a/pixl_dcmd/pyproject.toml +++ b/pixl_dcmd/pyproject.toml @@ -11,7 +11,7 @@ dependencies = [ "arrow==1.4.0", "deid==0.4.12", "dicom-anonymizer==2.0.0", - "dicom-validator==0.8.3", + "dicom-validator==0.9.0", "logger==1.4", "pydicom==3.0.2", "pydicom-data", diff --git a/uv.lock b/uv.lock index 74840dfc5..a57d8841b 100644 --- a/uv.lock +++ b/uv.lock @@ -735,16 +735,16 @@ wheels = [ [[package]] name = "dicom-validator" -version = "0.8.3" +version = "0.9.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "lxml" }, { name = "pydicom" }, { name = "pyparsing" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/dd/d7/7ae7a25d260613118f6d63545a2a0454aa209d06600c319ed9455a2d2887/dicom_validator-0.8.3.tar.gz", hash = "sha256:7b92f809f892ad37954e7b3c8c12bf10512496d6ae9a36db2dab3c1da456260e", size = 77652, upload-time = "2026-08-28T16:15:17.711Z" } +sdist = { url = "https://files.pythonhosted.org/packages/59/bd/467a5f6a2758bf87a78568a632207fc8788962b31f09300a9c329d5bd4eb/dicom_validator-0.9.0.tar.gz", hash = "sha256:1c9dc12da7646b8c4698253a80abb0151ce50d2b49bc483a3000c9a0acf835c8", size = 78818, upload-time = "2026-09-09T17:36:47.012Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/e1/17/c4fc7e4cf99a8c9217bc1a3d40f2996f3ddece11d22ce27e9bd220b29342/dicom_validator-0.8.3-py3-none-any.whl", hash = "sha256:755e800f05df8357b38902debbdb0588dd9206a281828fc788d80aa79a993927", size = 89443, upload-time = "2026-08-28T16:15:16.354Z" }, + { url = "https://files.pythonhosted.org/packages/d1/c4/790dd16e8fec78a7c6b7bf541414a29ba9ad526b4ee091b71a5082aa6434/dicom_validator-0.9.0-py3-none-any.whl", hash = "sha256:a89db814114a339c4ebe07e4c4e848b118078cc016169147fac051620d3cdae1", size = 90871, upload-time = "2026-09-09T17:36:45.46Z" }, ] [[package]] @@ -2150,7 +2150,7 @@ requires-dist = [ { name = "core", extras = ["test"], marker = "extra == 'test'", editable = "pixl_core" }, { name = "deid", specifier = "==0.4.12" }, { name = "dicom-anonymizer", specifier = "==2.0.0" }, - { name = "dicom-validator", specifier = "==0.8.3" }, + { name = "dicom-validator", specifier = "==0.9.0" }, { name = "logger", specifier = "==1.4" }, { name = "pydicom", specifier = "==3.0.2" }, { name = "pydicom-data" }, From 0480a126dcfa0cedbba789d0512d2b8dfb8d2242 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 16 Sep 2026 10:14:54 +0100 Subject: [PATCH 4/5] Update pixl_dcmd.dicom_helpers for dicom-validator 0.9.0 IODValidator.validate() now returns a ValidationResult class rather than a dictionary IODValidator.validate() no longer raises a RuntimeError if DICOM cannot be validated Use the ValidationResultFormatter class to conver the TagError class into a human readable string --- pixl_dcmd/src/pixl_dcmd/dicom_helpers.py | 124 ++++++++++++++--------- 1 file changed, 76 insertions(+), 48 deletions(-) diff --git a/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py b/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py index f5f8b7c70..b9a61216e 100644 --- a/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py +++ b/pixl_dcmd/src/pixl_dcmd/dicom_helpers.py @@ -19,7 +19,6 @@ import typing from contextlib import contextmanager, redirect_stdout from dataclasses import dataclass -import logging from io import StringIO from pathlib import Path from typing import Generator @@ -27,7 +26,19 @@ from loguru import logger from dicom_validator.spec_reader.edition_reader import EditionReader +from dicom_validator.tag_tools import tag_name_from_id +from dicom_validator.validator.error_handler import ( + NullValidationResultHandler, + ValidationResultFormatter, +) from dicom_validator.validator.iod_validator import IODValidator +from dicom_validator.validator.validation_result import ( + DicomTag, + ModuleErrors, + Status, + TagError, + ValidationResult, +) from pydicom import Dataset from core.exceptions import PixlSkipInstanceError @@ -46,60 +57,69 @@ def __init__(self, edition: str = "current"): edition_reader = EditionReader(standard_path) self.dicom_info = edition_reader.dicom_info_for_edition(self.edition) - def validate_original(self, dataset: Dataset) -> dict | None: + # Used to format errors introduced by de-identification + self.formatter = ValidationResultFormatter(self.dicom_info.dictionary) + + def _validate(self, dataset: Dataset) -> ValidationResult: + """Validate a pydicom Dataset using dicom-validator.""" + return IODValidator( + dataset, + self.dicom_info, + error_handler=NullValidationResultHandler(), + ).validate() + + def _describe_error(self, tag: DicomTag, error: TagError) -> str: + tag_name = tag_name_from_id(tag.tag, self.dicom_info.dictionary) + return f"Tag {tag_name}{self.formatter.error_message(error)}" + + def validate_original(self, dataset: Dataset) -> ModuleErrors | None: """Check pre-existing validation errors in a dataset. Returns: - validation_errors: a dictionary of validation errors, or None - if dicom-validator raised a RuntimeError during validation. + module_errors: pre-existing validation errors, keyed by module + name then DICOM tag, or None if dicom-validator could not + validate the dataset at all (e.g. missing or unrecognised + SOP Class UID). """ - validator = IODValidator( - dataset, - self.dicom_info, - log_level=logging.ERROR, - ) - try: - errors: dict | None = validator.validate() - except RuntimeError as error: + result = self._validate(dataset) + if result.status not in (Status.Passed, Status.Failed): logger.warning( "Cannot check for pre-existing validation errors. " - "dicom-validator raised a RuntimeError during validation: {}", - error, + "dicom-validator returned status: {}", + result.status, ) - errors = None + return None - return errors + return result.module_errors def validate_anonymised( - self, dataset: Dataset, original_errors: dict | None - ) -> dict: + self, dataset: Dataset, original_errors: ModuleErrors | None + ) -> dict[str, set[str]]: """Check validation errors introduced during de-identification. Args: - original_errors: dict of errors returned by validate_original for - dataset before anonymisation, or None if the dataset hasn't - been validated for pre-existing errors. + 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. Returns: - new_errors: dict of errors introduced by anonymisation. If - original_errors is None, all errors found after - anonymisation are returned, as it's not possible to tell - which of them pre-existed. + 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. Raises: - PixlSkipInstanceError: If dicom-validator raises a RuntimeError - during validation. + PixlSkipInstanceError: If dicom-validator could not validate the + anonymised dataset at all (e.g. missing SOP Class UID). """ - validator = IODValidator( - dataset, - self.dicom_info, - log_level=logging.ERROR, - ) - try: - anon_errors: dict = validator.validate() - except RuntimeError as error: - msg = f"dicom-validator raised a RuntimeError when validating the anonymised dataset: {error}" - raise PixlSkipInstanceError(msg) from error + result = self._validate(dataset) + if result.status not in (Status.Passed, Status.Failed): + msg = ( + "Cannot validate the anonymised dataset. " + f"dicom-validator returned status: {result.status}" + ) + raise PixlSkipInstanceError(msg) + anon_errors = result.module_errors if original_errors is None: logger.warning( @@ -108,18 +128,26 @@ def validate_anonymised( "Errors found after anonymisation: {}", anon_errors, ) - return anon_errors - - diff_errors: dict = {} - for key in anon_errors: - if key in original_errors: - # Keep only errors introduced after the anonymisation - # The keys of the dictionary containt the actual errors - diff = set(anon_errors[key]) - set(original_errors[key]) - if diff: - diff_errors[key] = diff + original_errors = ModuleErrors() + + diff_errors: dict[str, set[str]] = {} + for module_name, anon_tag_errors in anon_errors.items(): + if module_name in original_errors: + # keep tags with new errors or errors that have changed + original_tag_errors = original_errors[module_name] + new_tag_errors = { + tag: error + for tag, error in anon_tag_errors.items() + if (tag, error) not in original_tag_errors.items() + } else: - diff_errors[key] = anon_errors[key] + new_tag_errors = anon_tag_errors + + if new_tag_errors: + diff_errors[module_name] = { + self._describe_error(tag, error) + for tag, error in new_tag_errors.items() + } return diff_errors From d7dec18ab499c45ab33005dbf428804ac508c14f Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 16 Sep 2026 10:14:59 +0100 Subject: [PATCH 5/5] Update dcmd tests to be compatible with dicom-validator 0.9 --- pixl_dcmd/tests/test_dicom_validator.py | 65 +++++++++++++++---------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/pixl_dcmd/tests/test_dicom_validator.py b/pixl_dcmd/tests/test_dicom_validator.py index 112f2e483..121ad9471 100644 --- a/pixl_dcmd/tests/test_dicom_validator.py +++ b/pixl_dcmd/tests/test_dicom_validator.py @@ -16,6 +16,7 @@ import pytest from core.exceptions import PixlSkipInstanceError +from dicom_validator.validator.validation_result import ErrorCode from pixl_dcmd.dicom_helpers import DicomValidator from pixl_dcmd.main import anonymise_dicom from pydicom import Dataset @@ -85,10 +86,7 @@ def test_validation_fails_after_invalid_tag_modification( assert len(validation_result) == 1 assert "Patient" in validation_result.keys() assert len(validation_result["Patient"]) == 1 - assert ( - "Tag (0010,0010) (Patient's Name) is missing" - in validation_result["Patient"].keys() - ) + assert "Tag (0010,0010) (Patient's Name) is missing" in validation_result["Patient"] @pytest.fixture() @@ -103,21 +101,27 @@ def dicom_with_malformed_sequence_tag(vanilla_dicom_image_DX: Dataset) -> Datase return vanilla_dicom_image_DX -def test_validate_original_survives_runtime_error( +def test_validate_original_reports_malformed_sequence( dicom_with_malformed_sequence_tag: Dataset, ) -> None: """ - GIVEN a DICOM dataset that makes dicom-validator raise a RuntimeError + GIVEN a DICOM dataset with a malformed sequence tag WHEN the original dataset is validated - THEN None is returned rather than a dictionary of pre-existing errors + THEN an InvalidSequence error is returned """ validator = DicomValidator() original_errors = validator.validate_original(dicom_with_malformed_sequence_tag) - assert original_errors is None + + error_codes = { + error.code + for tag_errors in original_errors.values() + for error in tag_errors.values() + } + assert ErrorCode.InvalidSequence in error_codes def test_validate_anonymised_returns_all_errors_when_original_unknown( - dicom_with_malformed_sequence_tag: Dataset, + vanilla_dicom_image_DX: Dataset, ) -> None: """ GIVEN an anonymised dataset that has not been validated for pre-existing errors @@ -125,32 +129,41 @@ def test_validate_anonymised_returns_all_errors_when_original_unknown( THEN all errors found are returned """ validator = DicomValidator() - original_errors = validator.validate_original(dicom_with_malformed_sequence_tag) - assert original_errors is None - - # delete problematic element - del dicom_with_malformed_sequence_tag.DerivationCodeSequence - # delete a required element to introduce an error - del dicom_with_malformed_sequence_tag.PatientName + del vanilla_dicom_image_DX.PatientName - validation_result = validator.validate_anonymised( - dicom_with_malformed_sequence_tag, original_errors - ) + validation_result = validator.validate_anonymised(vanilla_dicom_image_DX, None) assert "Patient" in validation_result.keys() -def test_validate_anonymised_raises_skip_instance_error_on_runtime_error( - dicom_with_malformed_sequence_tag: Dataset, +@pytest.fixture() +def dicom_missing_sop_class_uid(vanilla_dicom_image_DX: Dataset) -> Dataset: + """A DICOM dataset with no SOP Class UID, which dicom-validator cannot validate.""" + del vanilla_dicom_image_DX.SOPClassUID + return vanilla_dicom_image_DX + + +def test_validate_original_returns_none_when_dataset_cannot_be_validated( + dicom_missing_sop_class_uid: Dataset, +) -> None: + """ + GIVEN a DICOM dataset that dicom-validator cannot validate at all + WHEN the original dataset is validated + THEN None is returned + """ + validator = DicomValidator() + original_errors = validator.validate_original(dicom_missing_sop_class_uid) + assert original_errors is None + + +def test_validate_anonymised_raises_skip_instance_error_when_dataset_cannot_be_validated( + dicom_missing_sop_class_uid: Dataset, ) -> None: """ - GIVEN an anonymised dataset that causes dicom-validator to raise a RuntimeError + GIVEN an anonymised DICOM dataset that dicom-validator cannot validate at all WHEN the anonymised dataset is validated THEN a PixlSkipInstanceError is raised """ validator = DicomValidator() - original_errors = validator.validate_original(dicom_with_malformed_sequence_tag) with pytest.raises(PixlSkipInstanceError): - validator.validate_anonymised( - dicom_with_malformed_sequence_tag, original_errors - ) + validator.validate_anonymised(dicom_missing_sop_class_uid, None)