Feature added: Get WSI at mpp
Fixes: #4980
Description
In this pull request, the feature to retrieve a whole slide image at a given mpp (microns per pixel) resolution was implemented for every WSIReader class in the function get_wsi_at_mpp.
While the implementations in the OpenslideWSIReader and CuCIMWSIReader classes were tested thoroughly, I could not find a suitable TIFF file for testing with the TiffFileWSIReader class.
For resizing, I have used PIL.Image.resize for Openslide and TiffFile, and cucim.sklearn.transform.resize for CuCIM. Originally, I used cv2.resize, but since the package isn't listed in requirements-dev.txt, I explored alternative solutions."
Types of changes
- [x] Non-breaking change (fix or new feature that would not break existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing functionality to change).
- [ ] New tests added to cover the changes.
- [x] Integration tests passed locally by running
./runtests.sh -f -u --net --coverage. - [x] Quick tests passed locally by running
./runtests.sh --quick --unittests --disttests. - [ ] In-line docstrings updated.
- [x] Documentation updated, tested
make htmlcommand in thedocs/folder.
Please let me know what you think and how I can improve this feature.
Best Niko
(Last PR was closed, because I changed the branch name to include the ticket id)
@NikolasSchmitz, I resolved the conflicts and updated the PR. Let's focus on the functionality and make this PR ready for reviewing. We can take care of any other issue once the PR is ready and please feel free to reach out to me or the working group if you still have any questions. Thanks for taking on this feature.
Thank you @drbeh ! I now marked it as ready for review.
@drbeh would you be able to review this? I think you're best qualified here. I made some minor comments about print and code duplication but otherwise it seems fine to me without having tested it. Thanks!
Thank you for the feedback, @JHancox . Unfortunately, I couldn't join the meeting because I was on my way home from ICLR. I will review the code and provide my updates as soon as possible.
I factored out the actual resizing of the image into _resize_to_mpp_res() and kept the tolerance checks in get_wsi_at_mpp(). I had some problems with specific tiff files and the TiffFile backend. Some files had zero as X/Y Resolution, which led to a division by zero error in the function get_mpp (added a check for that in the new commit). One tiff file from a Philips scanner that I tested had unobtainable mpp values using get_mpp. In this case, the tags do exist (openslide can find them), but they must be read and parsed from the property philips_metadata as XML. I have not implemented a fix for this yet, but please let me know if this is needed. In this particular case, the best option would be to use Openslide as the backend, although there might be users who prefer Tifffile.
There are lines added/removed to two other files that don't need to be changed in this PR, tests/test_regularization.py is causing a conflict for example. I would remove those and we can use black here through Github to resolve formatting issues if we need to.
There are lines added/removed to two other files that don't need to be changed in this PR, tests/test_regularization.py is causing a conflict for example. I would remove those and we can use black here through Github to resolve formatting issues if we need to.
Thanks for the info. Interesting though, haven't noticed that these files were changed. Perhaps through one of the coding style checks.
There's a few changes to do but it looks good in general. We do need tests for these new methods as well so please look at the existing WSI tests to see what needs to be added to cover new functionality. The flake8 and DCO fixes can be done at the end. Thanks!