imi-bigpicture / wsidicom

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

add_missing_levels does not recreate missing levels #97

Closed nstatho closed 1 year ago

nstatho commented 1 year ago

I used the add_missing_levels option to recreate the pyramid structure of DICOM file but it did not recreate the pyramid structure.

If I look into the code, in this line: https://github.com/imi-bigpicture/wsidicom/blob/90671211ddb87096d19952d5e80a7199dda51b67/wsidicom /file/wsidicom_file_target.py#LL79C1-L79C44

it seems that new_levels is set to "None" and then it never enters the "if" check later on.

erikogabrielsson commented 1 year ago

Hi @nstatho, Your observation is correct. Additionally the target-object is not used as a context in the WsiDicom.save() method so any new pyramid levels would be left open.

I have a draft for a fix here #100, but would like to test it more before merging.

erikogabrielsson commented 1 year ago

@nstatho Can you try the fix in the PR #100?

nstatho commented 1 year ago

I tried it and it seems to work. I tested it on several DICOMs and all except one gave an error. The error I got from that one DICOM seems to be related to that one specific file.

This is the error: Frame count 2040 != Fragments 2023. Fragmented frames are not supported

erikogabrielsson commented 1 year ago

Thanks for the feedback @nstatho. I assume you mean that all worked except one that gave the error?

Any chance you can share the failing file? I have not yet encountered a DICOM WSI with fragmented frames.

nstatho commented 1 year ago

Yes it worked with all except one indeed. I'll send you the file on a separate email to take a look. If I view the file in PACS, you see black and white squares on certain magnifications so I think the file itself has an issue.

erikogabrielsson commented 1 year ago

The second and third layer seems to be missing tiles on the bottom row. For example, dataset.NumberOfFrames returns 2040 but len(dataset.PerFrameFunctionalGroupsSequence) returns 2023.

nstatho commented 1 year ago

Yeah so I removed them and I applied the missing levels method and it worked. So that's +1 for the PR fix.

On Tue, May 9, 2023, 5:00 PM Erik O Gabrielsson @.***> wrote:

The second and third layer seems to be missing tiles on the bottom row. For example, dataset.NumberOfFrames returns 2040 but len(dataset.PerFrameFunctionalGroupsSequence) returns 2023.

— Reply to this email directly, view it on GitHub https://github.com/imi-bigpicture/wsidicom/issues/97#issuecomment-1540313555, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEOPKIB3BRS7BQDOAMNT2TXFJLZZANCNFSM6AAAAAAXV4MK5I . You are receiving this because you were mentioned.Message ID: @.***>