Fix row-column/X-Y coordinates mix-up in Random Tiler

[alessiamarcolini]

Description

This PR fixes the mismatch between row-column / X-Y coordinates in the RandomTiler. In the end, the result was correct due to a double bug, but know they should both be fixed.

Moreover, it fixes inverted y_ul and x_br in CoordinatePair used in unit testing (causing no harm), and it moves the conversion from np coords to PIL coords to regions_from_binary_mask

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Documentation

Issues Fixed or Closed by This PR

• Fixes: Row-column X, Y coordinate mix-up in RandomTiler #309

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[codecov]

Codecov Report

Merging #317 (7fe6469) into master (fbfc897) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #317   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         1284      1285    +1     
  Branches       124       124           
=========================================
+ Hits          1284      1285    +1     

Impacted Files	Coverage Δ
src/histolab/tiler.py	100.00% <ø> (ø)
src/histolab/util.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbfc897...7fe6469. Read the comment docs.

[ernestoarbitrio]

I like the fix ❤️ thx

Some comments before i can do another review round :D

[ernestoarbitrio]

did you consider the performance during testrunner increasing the n tiles of 2 order of magnitude?

[alessiamarcolini]

I did consider it, but the point is that with only 2 tiles we cannot really see how the tiler is sampling the tiles, if you compare the old and new expectations you will see that only now this test is really meaninfgul

[alessiamarcolini]

@ernestoarbitrio to do what you suggest I need to unroll the list comprehension to a normal for statement.. I will do that

[ernestoarbitrio]

@ernestoarbitrio to do what you suggest I need to unroll the list comprehension to a normal for statement.. I will do that

Actually i think you can still use comprehension if you write an internal function that given the rp.bbox it gives you back the correct new bbox shape. Hope i have been clear

[alessiamarcolini]

actually this code is wrong..

bbox = list(reversed(rp.bbox))
and then in the Region pass bbox=[*bbox[:2], *bbox[2:]]

I need to "split" the tuple in two and then individually reverse.. anyway I will implement it in a subfunction

It would be like this in the end bbox_pil = [*reversed(bbox_np[:2]), *reversed(bbox_np[2:])].

[ernestoarbitrio]

LGTM, just 1 minor thing. Good to go after this

[kheffah]

Thanks @alessiamarcolini for this important PR! All looks good to me.