scikit-image
scikit-image copied to clipboard
Add offset to regionprops
Description
To calculate region properties in a bigger volume than can be held in one image, we need to be able to specify the coordinates of the input image as an input to the regionprops function. Amazingly, as far as I can tell, this only affects the "coordinates" and "centroid" properties.
Checklist
- [x] Docstrings for all functions
- [ ] ~~Gallery example in
./doc/examples
(new features only)~~ (Note: this will help dask-image, less helpful here...) - [ ] ~~Benchmark in
./benchmarks
, if your changes aren't covered by an existing benchmark~~ - [x] Unit tests
- [x] Clean style in the spirit of PEP8
For reviewers
- [ ] Check that the PR title is short, concise, and will make sense 1 year later.
- [ ] Check that new functions are imported in corresponding
__init__.py
. - [ ] Check that new features, API changes, and deprecations are mentioned in
doc/release/release_dev.rst
. - [ ] Consider backporting the PR with
@meeseeksdev backport to v0.14.x
Note: this doesn't quite address this SO question, but it gets us partway there.
Why not just slice the image before computing regionprops?
You slice it, then you can manually add in the slice start indices. Or you can slice it and add the appropriate offset=
and get correctly-computed props in the global coordinate system.
Does this seem reasonable @stefanv? @jni and I were interested in this because we were thinking how to handle a Dask-based regionprops
implementation leveraging scikit-image under-the-hood.
@jni This looks good to me (pending resolving the merge conflict and maybe rebasing on main). I am thinking about releasing 0.20.0rc0 tomorrow and wanted to check whether you thought we should merge this before or after 0.20. If before, feel free to make the updates and merge once the tests pass.
@jni I'm afraid we need some test updates too.
Sorry, I got distracted and wasn't able to investigate at the time. I've fixed the issue (with a force, had done the same thing and was easier than trying to do a bunch of pulling/merging/cherry-picking, sorry and thank you @jakirkham and @stefanv!), but there's a bit of an API design issue too, which is, self.slice in some use cases would itself have to include the offset. Just like we have coords
and coords_scaled
, we also might want slice
and slice_offset
. For example, if one did regionprops
in a dask array, we would want to have the slice objects be valid not on chunks, but on the whole image.
However, there's several steps between this PR and dask_image.measure.regionprops. For example, we need to be able to combine features between chunks. So I don't think that this decision should hold up merge — this is already very useful to start experimenting with, I think.
Thanks @jni! Feel free to open an issue for anything you still want to address.
Thanks all for shepherding this one in! 😄
👋 @jakirkham long time no see. 😊 let me know if you want to pair up on dask_image regionprops any time soon. 😃