imi-bigpicture / wsidicom

Python package for reading DICOM WSI file sets.
Apache License 2.0
30 stars 5 forks source link

Difference behaviour between WsiDicomFileSource and WsiDicomWebSource #163

Open ChristianMarzahl opened 1 month ago

ChristianMarzahl commented 1 month ago

Hi,

First of all, thank you very much for providing this great library.

I noticed a difference in the handling of the same DICOM file provided by WsiDicomFileSource and WsiDicomWebSource through a DICOMWeb interface.

The WsiDicomFileSource filters the DICOM levels for matching tile_size and UIDs: https://github.com/imi-bigpicture/wsidicom/blob/b0d097c97140ba595529d60de43c472ffc5c6d6b/wsidicom/file/wsidicom_file_source.py#L274

This filtering is not done in WsiDicomWebSource: https://github.com/imi-bigpicture/wsidicom/blob/b0d097c97140ba595529d60de43c472ffc5c6d6b/wsidicom/web/wsidicom_web_source.py#L58

This discrepancy can lead to DICOMs that can be opened via file but not via the web interface. I wanted to ask if this was intended.

Regarding the same topic of opening images via file or DCMWeb: https://github.com/imi-bigpicture/wsidicom/blob/b0d097c97140ba595529d60de43c472ffc5c6d6b/wsidicom/web/wsidicom_web_source.py#L102

If the DICOMWeb server does not support JPEG2000, for example, as a transfer syntax and throws an exception (requests.exceptions.HTTPError: 400 Client Error: BAD REQUEST for url: Google DICOM Archive), the code only catches 406 exceptions.

https://github.com/imi-bigpicture/wsidicom/blob/b0d097c97140ba595529d60de43c472ffc5c6d6b/wsidicom/web/wsidicom_web_client.py#L218

With kind regards, Christian Marzahl

erikogabrielsson commented 2 weeks ago

Hi @ChristianMarzahl Thanks for the feedback and sorry for the late response.

Limiting WsiDicom to only handle pyramid levels of same tile size was an early design decision. I'm not sure if it is still needed, if it with some effort the restriction could be removed, or how common it is with DICOM WSI-files with non-uniform tile size. Handling WSIs with non-uniform tile size differently when opened by DICOM Web was not intended.

I can try to create some test files with non-uniform tile size, but it would also be helpful to know where the library fails when you open your example as DICOM Web.

For the second issue, my interpretation of the standard is that the server should respond with 406 (Not Acceptable) when the requested transfer syntax is not accepted. Does the more generic 400 (Bad Request) response from Google DICOM Archive give a clear status reason that can be used to check if the error is due to the requested transfer syntax?

erikogabrielsson commented 2 weeks ago

I played around a bit with disabling the tile size check together with a slide with levels with tile sizes of 200, 300, and 400 pixels. read_region() works as expected but it gets a bit weird when using get_tile() as (of coarse) the tiles will be of different size.

In the images that you have encountered, what levels are of different tile size? Is relaxation to allow different tile sizes a wanted feature or is it better to drop them (and implement the same checks for the DICOM web source)?

erikogabrielsson commented 6 days ago

@ChristianMarzahl I have started working on this in this branch.

Please try this on your problematic WSI.

Also please provide the HTTP error message for Google DICOM archive on unsupported transfer syntax.

ChristianMarzahl commented 6 days ago

Dear @erikogabrielsson,

Thank you very much for looking into it. I hope to spend some time on it this week to answer your questions.

Best, Christian