Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/dicognito/release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

- Drop support for Python 3.10 ([#185](https://github.com/blairconrad/dicognito/issues/185))

### Fixed

- Anonymized UIs are not conformant DICOM UIDs. They now use the UUID-derived "2.25." form ([#189](https://github.com/blairconrad/dicognito/issues/189))

## 0.19.0

### Changed
Expand Down
9 changes: 8 additions & 1 deletion src/dicognito/uianonymizer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Replace UIs with a new value."""

import uuid
from collections.abc import Iterator

import pydicom
Expand All @@ -8,6 +9,10 @@
from dicognito.element_anonymizer import ElementAnonymizer
from dicognito.randomizer import Randomizer

# Fixed namespace for dicognito's name-based UUIDs. Never change it, or the same
# seed will stop producing the same UIs.
DICOGNITO_NAMESPACE = uuid.UUID("220d81d1-0385-470f-9194-f7c213aa7490")


class UIAnonymizer(ElementAnonymizer):
"""
Expand All @@ -19,6 +24,7 @@ class UIAnonymizer(ElementAnonymizer):
def __init__(self, randomizer: Randomizer) -> None:
"""Create a new UIAnonymizer."""
self._randomizer = randomizer
self._seed_namespace = uuid.uuid5(DICOGNITO_NAMESPACE, randomizer.seed)

def __call__(
self,
Expand Down Expand Up @@ -62,4 +68,5 @@ def describe_actions(self) -> Iterator[str]:
yield "Replace all UI elements with anonymized values"

def _new_ui(self, ui: str) -> str:
return "2." + str(10**39 + self._randomizer.to_int(ui))
# A UID made from a UUID, as described in DICOM PS3.5 section B.2.
return f"2.25.{uuid.uuid5(self._seed_namespace, ui).int}"
57 changes: 57 additions & 0 deletions tests/test_anonymize_single_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

from .data_for_tests import load_dcm, load_minimal_instance, load_test_instance

MAX_UI_LENGTH = 64


def test_minimal_instance_anonymizes_safely():
with load_minimal_instance() as dataset:
Expand Down Expand Up @@ -78,6 +80,61 @@ def test_repeated_identifying_uis_get_same_values(one_element_path, another_elem
assert one_uid == another_uid


@pytest.mark.parametrize(
"element_path",
[
"file_meta.MediaStorageSOPInstanceUID",
"SOPInstanceUID",
"StudyInstanceUID",
"SeriesInstanceUID",
"FrameOfReferenceUID",
],
)
def test_identifying_uis_are_conformant_uuid_derived_uids(element_path):
with load_test_instance() as dataset:
anonymizer = Anonymizer()
anonymizer.anonymize(dataset)

actual = eval("dataset." + element_path)

assert actual.startswith("2.25.")
assert len(actual) <= MAX_UI_LENGTH

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figure why make up a constant we have to maintain.
pydicom.valuerep.MAX_VALUE_LEN["UI"] is available, but an alternative, which I like, is to check that the remainder represents an int with no leading 0 and that int is a valid uuid5. It's a tighter check than DICOM requires, but would keep us from surprises. Also, no need for a length check then.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, that's a nicer check. I dropped the constant and the length assert. The test now checks that everything after "2.25." is digits with no leading 0 and decodes to a version 5 UUID, and the multivalued test uses the same helper.

assert pydicom.uid.UID(actual).is_valid


def test_multivalued_identifying_ui_gets_conformant_values():
with load_test_instance() as dataset:
original = ["1.2.3.1.1", "1.2.3.1.2"]
dataset.SourceImageSequence[0].ReferencedSOPInstanceUID = original

anonymizer = Anonymizer()
anonymizer.anonymize(dataset)

actual = dataset.SourceImageSequence[0].ReferencedSOPInstanceUID

assert len(set(actual)) == len(original)
for value in actual:
assert value.startswith("2.25.")
assert pydicom.uid.UID(value).is_valid


def test_identifying_uis_are_the_same_with_the_same_seed():
with load_test_instance() as dataset1, load_test_instance() as dataset2:
Anonymizer(seed="some seed").anonymize(dataset1)
Anonymizer(seed="some seed").anonymize(dataset2)

assert dataset1.SOPInstanceUID == dataset2.SOPInstanceUID
assert dataset1.StudyInstanceUID == dataset2.StudyInstanceUID


def test_identifying_uis_differ_with_a_different_seed():
with load_test_instance() as dataset1, load_test_instance() as dataset2:
Anonymizer(seed="some seed").anonymize(dataset1)
Anonymizer(seed="another seed").anonymize(dataset2)

assert dataset1.SOPInstanceUID != dataset2.SOPInstanceUID


@pytest.mark.parametrize(
"element_path",
[
Expand Down