KitwareMedical / dicom-anonymizer

Tool to anonymize DICOM files according to the DICOM standard
BSD 3-Clause "New" or "Revised" License
104 stars 47 forks source link

anonymize_dataset fails if dataset contains RawDataElement #85

Open GuillaumeDehaene opened 5 months ago

GuillaumeDehaene commented 5 months ago

Hello,

Thank you for this great project.

While using it in our codebase, I have found the following issue.

anonymize_dataset fails if dataset contains RawDataElement

anonymize_dataset fails if the dataset contains RawDataElement:

Proposed solution

I would be happy to create a PR to fix this issue here too.

I see two possibilities:

  1. Sanitize dataset as part of anonymize_dataset:
    • iterate over the dataset and replace RawDataElements,
    • continue with the current code of anonymize_dataset.
  2. Modify current replace_element code to handle the RawDataElement case.

I'm partial to solution 1.:

Best Guillaume

pchoisel commented 3 months ago

Hi @GuillaumeDehaene, please forgive me for the lack of reactivity, I now have a bit of time to work on this project.

I understand the problem but I'm wondering how can this problem happen ? Do those RawDataElement come directly from DICOM files or are you anonymizing a custom dataset ?

To fix this issue, instead of parsing the dataset twice to sanitize it, couldn't we just drop the tag if we detect that it is a RawDataElement ? This could be done in dicomanonymizer/simpledicomanonymizer.py::anonymize_dataset()

GuillaumeDehaene commented 3 months ago

Hello @pchoisel

No worries. Thank you for maintaining this project.

So this was coming from a real DICOM dataset, but I'm not in contact with the person who generated it so I don't have any info regarding which software, which kind of data, etc. I also was unable to figure out what this RawDataElement corresponds to in pydicom 🤷

I guess we could also throw out any raw data but that feels very rough, doesn't it? Double-parsing the data is suboptimal but it is also simple. Are you so worried about the performance hit?

Anyway, I've made my case and I think those are the options. If you let me know which option you prefer, I'll write a PR for it and we can discuss how to proceed further on that basis.

Thank you again for your work on this project. Best regards Guillaume

pchoisel commented 3 months ago

Hi @GuillaumeDehaene,

I definitely don't want to parse the dataset twice. That's mainly because people (at least us) use this software in automated processes that anonymize lots of DICOM files.

I'm fine with not dropping the RawDataElement. Have you checked the function DataElement_from_raw ? Maybe we could use it to handle the RawDataElement. If you want to keep those tags as RawDataElement, maybe you can re-transform them before writing the output file ?

What do you think ?

GuillaumeDehaene commented 3 months ago

Hello

If you don't want to double-parse, then it's probably possible to rewrite the parsing code to:

I'll write it out. I'll try to have a draft done for end of september, hopefully?

Best Guillaume

Le lun. 5 août 2024 à 15:43, pchoisel @.***> a écrit :

Hi @GuillaumeDehaene https://github.com/GuillaumeDehaene,

I definitely don't want to parse the dataset twice. That's mainly because people (at least us) use this software in automated processes that anonymize lots of DICOM files.

I'm fine with not dropping the RawDataElement. Have you checked the function DataElement_from_raw https://pydicom.github.io/pydicom/dev/reference/generated/pydicom.dataelem.DataElement_from_raw.html ? Maybe we could use it to handle the RawDataElement. If you want to keep those tags as RawDataElement, maybe you can re-transform them before writing the output file ?

What do you think ?

— Reply to this email directly, view it on GitHub https://github.com/KitwareMedical/dicom-anonymizer/issues/85#issuecomment-2269113347, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZDZEDBGBWYSTGI2OUIE53ZP56OPAVCNFSM6AAAAABIKXCF2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRZGEYTGMZUG4 . You are receiving this because you were mentioned.Message ID: @.***>

pchoisel commented 2 months ago

That sounds perfect !