Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Anonymize names of non-UID tags #94

Closed

Conversation

bpycinski
Copy link
Contributor

Currently gdcmanon does not create dummy names of Patient's Name, Patient ID, Study ID and Series Number, despite of these tags are explicitly listed to be prevented (see gdcmAnonymizer.cxx lie 761 and 804-809). This commit fixes incoplete initialization of a dictionary dummyMapNonUIDTags.

Behavior of gdcmanon --de-identify before the patch:

$ gdcmdump outfile_orig | grep -E '0010,0010|0010,0020|0020,0010'
(0010,0010) PN (no value)                                         # 0,1 Patient's Name
(0010,0020) LO (no value)                                         # 0,1 Patient ID
(0020,0010) SH (no value)                                         # 0,1 Study ID

After the patch:

$ gdcmdump outfile_patch | grep -E '0010,0010|0010,0020|0020,0010'
(0010,0010) PN [3b304717c479a4272b707048dbe7ee63]                 # 32,1 Patient's Name
(0010,0020) LO [5d82026296e8b502d8eb9c5fa87d5b12]                 # 32,1 Patient ID
(0020,0010) SH [a569398d13eb87b14276380d2921ddbc]                 # 32,1 Study ID

@malaterre
Copy link
Owner

@bpycinski Thanks for the reminder ! The reason I am not merging this directly is that I never liked the fact that I was producing invalid content for some Value Representation. I vaguely even recall reporting a bug about that saying something like: 'we need a better random generator, one for each VR'. So I'll leave this PR aside for now. Thanks.

@bpycinski
Copy link
Contributor Author

Salut Mathieu! Thank you for your explanation. The comments in the source as well as the presence of unnecessary variable DummyMapNonUIDTags did mislead me, so I assumed that it had been a bug. But now it is clear to me.

@bpycinski bpycinski closed this Jun 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants