Update imagewriter.py with tile and coordinate checks and corrections

DIAGNijmegen / pathology-whole-slide-data

A package for working with whole-slide data including a fast batch iterator that can be used to train deep learning models.

https://diagnijmegen.github.io/pathology-whole-slide-data/

Apache License 2.0

93 stars 27 forks source link

Update imagewriter.py with tile and coordinate checks and corrections #65

Closed JoeySpronck closed 1 month ago

JoeySpronck commented 2 months ago

As discussed with multiple people from our group the wrapped ASAP image writer sometimes exhibits unexaplainable behaviour that is very hard to debug. In this PR I intend to prevent some of the obvious mistakes to pass without errors. If possible I correct the tile. By doing this the writer becomes more robust and guides the user on what to fix, instead of ambiguously trying to keep the loop going without throwing warnings or errors.

I think the _crop_tile function and the _get_row_col function already partly do some of these checks, but neither throw errors or warnings, causign outputs that are incorrect and leaving users unaware of what went wrong. I left them in right now though, the new function takes care of possible errors.

I havent tested this yet but I would like to share this already so we can finalize it together. @martvanrijthoven @daangeijs @carlijnlems @nfsuysal @leandervaneekelen @rolandnemeth000

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 11382794478

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

12 of 18 (66.67%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.04%) to 72.629%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
wholeslidedata/interoperability/asap/imagewriter.py	12	18	66.67%
<!--	Total:	12	18	66.67%	-->

Totals
Change from base Build 10526088141:	-0.04%
Covered Lines:	2221
Relevant Lines:	3058

💛 - Coveralls

JoeySpronck commented 1 month ago

Hey @martvanrijthoven, I adjusted the PR. One check/adjustment wasnt needed, I removed it. All proposed check now are pretty much straightforward and will prevent ambiguous/incorrect behaviour. Also guides the user in adjusting errors.

martvanrijthoven commented 1 month ago

Hey Joey,

Super nice looks good, would you say its ready for merge?

One small note, I am not a big fan of the (my opinion: redundant) comments above already self explanatory code you wrote. But up to you to remove or keep them.

JoeySpronck commented 1 month ago

@martvanrijthoven Agreed about the comments, removed them, ready for merge!