search

pydicom / deid

best effort anonymization for medical images using python
https://pydicom.github.io/deid/
MIT License
140 stars 43 forks source link

generate_uid example should return a value that conforms to the DICOM VR UI character repertoire #130

Closed sjswerdloff closed 3 years ago

sjswerdloff commented 4 years ago

https://github.com/pydicom/deid/blob/master/examples/dicom/header-manipulation/func-replacement.py#L98 uses uuid and a str representation of that. (e.g. 022f82f4-e9df-4533-b237-6ab563dfaf56 )

But the DICOM UI has a different character repertoire. "0"-"9","." of Default Character repertoire. There's a lot more detail in the standard, e.g. no leading zeros.

for example: "1.2.840.10008.1.1" (no double quotes, the ubiquitous ECHO service SOP Class UID).

An example of how to do this is in https://gist.github.com/jaime-olivares/ee91edd02809abe62bfd899fb9be74c0 in a responding comment with a function (in C#?) called ConvertGuidToDicomUid(ref Guid value)

vsoch commented 4 years ago

That would be a good update, as long as it’s not too complicated so it’s still a dummy example (keep this in mind, I’m not showing how to generate some actual important thing, it was just a random dummy example). Would you care to open a PR with a suggestion?

sjswerdloff commented 4 years ago

I created a PR, but it's really intended as a straw man/example. I could only do it against master, and not development (development didn't have a lot of the files, there was merge conflict, etc), so I couldn't adhere to that part of the Pull Request process.

JJdeRidder commented 3 years ago

In https://github.com/pydicom/deid/blob/master/examples/dicom/header-manipulation/func-replacement.py#L106 a root of an organization is used to generate the UID. I think you can also just use the root '2.25' for a UID created from a UUID, as specified in the standard. As long as you use it for dynamically created UIDs.

vsoch commented 3 years ago

@JJdeRidder I think @sjswerdloff added his update here: https://github.com/pydicom/deid/pull/131 and your point is a good one too! Do you want to add this note to the script / documentation for future folks to find? After that, we can close the issue.

vsoch commented 3 years ago

We've addressed both things here, closing issue! Thank you to you both!