search

ImagingDataCommons / highdicom

High-level DICOM abstractions for the Python programming language
https://highdicom.readthedocs.io
MIT License
168 stars 35 forks source link

Provide always the available metadata #187

Closed Eschivo closed 2 years ago

Eschivo commented 2 years ago

I recently came across this SO question and as a solution, I answered suggesting the usage of this package. However, I noticed that since the attributes ICCProfile and OpticalPathSequence are not present, it's not possible to inspect the metadata of the file provided by the OP at all. Furthermore, since the code works correctly with the correct_color option set to False in read_frame, why not print a warning when reading metadata instead of raising the AttributeError and raise it subsequently in the read_frame function if correct_color is True? I think that it's better to have the possibility to inspect, view and print all the available metadata rather than stop the code execution if one is missing. As a disclaimer, I'm not a DICOM expert so maybe there's a reason to do the things in this way that I can't actually understand.

hackermd commented 2 years ago

@EliaSchiavon thank you very much for answering the question on SO and for reporting the issue. I agree with your assessment. It is probably better for highdicom.io.ImageFileReader to not fail upon construction to allow reading of image frames without color correction.

189 addresses the issue. I tested it on the example image provided in the SO question. Could you please give it a try, too?

Eschivo commented 2 years ago

I'm seeing this only now, I'll give it a try anyway today. Thank you for your work!

CPBridge commented 2 years ago

@hackermd did you mean to merge #189 ? it has not been merged yet?