Fix writing pyramid - Githubissues

DIAGNijmegen / rse-panimg

Conversion of medical images to MHA and TIFF.

Apache License 2.0

13 stars 5 forks source link

Fix writing pyramid #103

Closed miriam-groeneveld closed 10 months ago

miriam-groeneveld commented 11 months ago

Not every level was written with the correct data. Creating dzi files seems to have obfuscated the issue on GC.

miriam-groeneveld commented 11 months ago

OpenSlide now supports DICOM, which I think is a much better solution than writng our own converter. These changes are not part of libvips yet. We can either:

build libdicom, openslide and libvips from git master, as mentioned here: https://github.com/libvips/libvips/issues/1225. I've asked for an update on the expected release date.
use openslide directly for the conversion
wait for libvips (expectd to be added end of September)

I'll add an update once John Cupitt has replied, and just add the OpenSlide conversion if no answer by Monday.

pkcakeout commented 11 months ago

I honestly think that building libvips ourselves might be a better solution, especially because it seems clear that they will ultimately add the functionality. What I would do is to add the build-code right here in this repository so that rse-panimg still builds from sources. I can help with that if you want.

The disadvantage of adding openslide is that we add another hard dependency that we need to maintain ourselves. If we only rely on libvips our dependency graph stays leaner, with a short-term pain of having to maintain the build system.

Note: I built libvips in the past and know that it is not super-bad to do that. So I think that this is very much an option. Could probably get this working in a day or so...

pkcakeout commented 11 months ago

From a functional/implementation side: adding openslide directly is not a bad solution either of course!

miriam-groeneveld commented 10 months ago

We already use openslide for the other pathology > tiff conversions (like mrxs, vmu, etc.), so it wouldn't be a new dependency.

pkcakeout commented 10 months ago

I wanted to come back at this one as well and say: Whatever, libvips might be "ideal" but I am overthinking this again. And if we use it already, it is even better! :D

miriam-groeneveld commented 10 months ago

This PR became a mess, I opened a new one with at least a fix for the issue: https://github.com/DIAGNijmegen/rse-panimg/pull/104