search

ImagingDataCommons / highdicom

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

Handle DicomFileLike #223

Closed RobinFrcd closed 1 year ago

RobinFrcd commented 1 year ago

Fixes https://github.com/ImagingDataCommons/highdicom/issues/222

Not sure about the expected behavior of _check_file_format as fp wasn't even used.

CPBridge commented 1 year ago

Hi @RobinFrcd , thanks for raising this issue and providing a solution. I'm going to need to find some time to dig into this since I'm not totally clear on what the intention was when this was written. Would be great if @hackermd can shed some light if he has time.

Out of interest, what is your goal with using the ImageFileReader here? The typical use case is one where you do not wish to read the entire file into memory because it may be very large. But in your case the object is already in memory as it is a BytesIO. Are you trying to avoid the cost of decompressing the full set of frames?

CPBridge commented 1 year ago

My guess is that the intention was that a DICOM file handle that is open (but not read into memory) could be passed as the argument. We would want to continue to support that case. Probably this means that the case of a DICOMBytesIO would be a further special case in the if...else chain. I agree that it would be a useful addition but it should not replace the existing case of an open file handle, and the logic for the two is somewhat different.

Just my initial thoughts, need to take a better look

RobinFrcd commented 1 year ago

Out of interest, what is your goal with using the ImageFileReader here? The typical use case is one where you do not wish to read the entire file into memory because it may be very large. But in your case the object is already in memory as it is a BytesIO. Are you trying to avoid the cost of decompressing the full set of frames?

My use case here is that I download a DICOM file from a server and don't want to write it to disk. This file is 25MB on disk, 160MB on RAM when read by pydicom and 730MB if I load pixel_data ! So I want to keep the bytes on RAM and only read the frames I need with highdicom !

I'll add a if else then !

RobinFrcd commented 1 year ago

I've restored the previous case for DicomFileLike, all the tests are still passing :+1:

RobinFrcd commented 1 year ago

Hi, Any news on this one ? I've been using my fork for a week now, working as expected ! 👍

CPBridge commented 1 year ago

There are some small linter errors to deal with: https://github.com/ImagingDataCommons/highdicom/actions/runs/4689483856/jobs/8311389950?pr=223 @RobinFrcd

CPBridge commented 1 year ago

Ok I'll merge. Thanks @RobinFrcd , we'll aim to have this in an upcoming patch release, probably after we fix #225