rordenlab / dcm2niix

dcm2nii DICOM to NIfTI converter: compiled versions available from NITRC
https://www.nitrc.org/plugins/mwiki/index.php/dcm2nii:MainPage
Other
898 stars 229 forks source link

DeidentificationMethod issues #877

Closed neurolabusc closed 4 weeks ago

neurolabusc commented 1 month ago

The development branch adds DeidentificationMethod fields to meet the latest BIDS specification. The dcm_qa_deident repository provides a concrete example.

The current implementation has several weaknesses (including some detected by @captainnova):

  1. This new feature re-reads DICOM headers (to reduce memory pressure). This should only occur if the file includes de-identification fields, removing the speed penalty for conversions without these values.
  2. When re-reading, there should be a check that the DICOM file still exists. This might also be elicited by weird filenames and file systems.
  3. All deidentification strings should be explicitly set to zero length to avoid overflow errors. The number of items does not predict whether all items have all strings. For example, in the validation dataset, the CodingSchemeVersion (0008,0103) is a property of some but not all DeidentificationMethodCodeSequence items.
captainnova commented 4 weeks ago

I encountered bogus DeidentificationMethods for some series if writing to /tmp, but this seems to be a (still undiagnosed) property of my build, and may not have much to do with /tmp being XFS. The input was never in /tmp, and even the output files were not really temporary, so the cause is a mystery.

However, I think the recent changes made for 2. and 3. are good ideas, and they fix the problem.