moloney / dcmstack

DICOM to Nifti conversion with meta data preservation
Other
72 stars 51 forks source link

Enh/srcfilenames #2

Closed satra closed 11 years ago

satra commented 11 years ago

adds source dicom filename to stack object.

moloney commented 11 years ago

I am having a hard time understanding what the purpose of this change is. Could you explain the goal?

satra commented 11 years ago

the goal is to remember which dicom file name the stack object was created from. i would ideally also like to hash the dicom files into the stack object and into the metadata.

moloney commented 11 years ago

I see that, but the filenames are just being stored in the private "_files_info" attribute of the stack. There isn't any way for users to access this information through the public methods/attributes.

Perhaps it would make more sense to store the filename and hash in the meta dictionary for each file? One potential issue here is dealing with PHI in the filename.

moloney commented 11 years ago

Would it solve your use case if parse_and_stack returned a list of failed file names? I think this makes the most sense since the DicomStack object does not deal with file names itself.

Edit: I suppose this still wouldn't tell you which dicom files ended up in which stack. I think this indicates that we need a separate function for just parsing and grouping the dicom files.

satra commented 11 years ago

yes a separate function for parsing and grouping dicom files would address my issue, as long as i could maintain a link between the series grouping and the dicomstack objects. for example, the following functions would work :

def parse_and_group_files(dicomdir/files):
   return dictionary

def create_stack_objects(dictionary, anonymization_protocol=None)
    return stackobjects

perhaps even allow for different anonymization protocols for each object.

moloney commented 11 years ago

How does this look: https://github.com/moloney/dcmstack/commit/911aa58ca0b96962248010578062f0599baf861e

Also, anonymization can be done with a custom extractor (to the parse_and_group function). Or if your anonymization can work with just the DICOM keywords you could use the 'meta_filter' argument passed to the DicomStack constructor.

satra commented 11 years ago

sorry for the delay, this change looks good. would you prefer anonymization functionality not to be added to dcmstack?

@ssikka implemented most of the ctp protocol here: https://github.com/ssikka/DICOM-CTP-Anonymizer

i think a cleaned-up version would be very useful in the context of dcmstack

satra commented 11 years ago

@moloney could we merge 911aa58 and close this?

moloney commented 11 years ago

Ok the changes have been merged, so I will go ahead and close this.

Would be happy to see the anonymization functionality in dcmstack improved/exteneded. I would just like it to remain flexible enough that folks can plug in their own anonymization if they like.