Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weighted region properties #3666

Open
anntzer opened this issue Jan 20, 2019 · 3 comments
Open

Weighted region properties #3666

anntzer opened this issue Jan 20, 2019 · 3 comments
Labels
⏩ type: Enhancement Improve existing features 📜 type: API Involves API change(s) 💬 Discussion

Comments

@anntzer
Copy link
Contributor

anntzer commented Jan 20, 2019

Description

It seems a bit unfortunate that regionprops use unweighted properties by default even when the intensity_image is passed, and that one needs to pay attention to the entire docstring to realize that one needs to use the prop.weighted_foo variants instead of prop.foo to get the weighte centroid, etc. Moreover, certain properties (orientation, major/minor_axis_length) are not available in the weighted form (obviously I can compute them manually from the weighted moments...).

If backcompat was not a concern, I would think that prop.centroid, etc. should clearly have referred to the weighted variants when the intensity image is passed as argument; but I guess this ship may have sailed. Instead, I would suggest adding a clear note next to the doc for the intensity_image parameter pointing to the prop.weighted_foo properties, and also filling in the missing weighted properties indicated above.

@jni
Copy link
Member

jni commented Feb 1, 2019

Well, yes, backcompatibility is indeed a major concern for us, and anyway, I think people generally think of orientation etc as relating to the unweighted image. But weighted variants of this, as well as updates to the documentation, would be welcome contributions. Otherwise we'll get to them eventually. =)

@anntzer
Copy link
Contributor Author

anntzer commented Feb 1, 2019

I would think that if someone bother passing in an intensity array, it is not for it to be ignored, but heh.

@anntzer
Copy link
Contributor Author

anntzer commented Jun 11, 2019

I guess one could just have a weighted_regionprops instead of duplicating each and every regionprop (and then the nonweighted regionprops could be implemented in terms of the weighted one).

@grlee77 grlee77 added the ⏩ type: Enhancement Improve existing features label Aug 1, 2020
@scikit-image scikit-image locked and limited conversation to collaborators Oct 18, 2021
@grlee77 grlee77 added 📜 type: API Involves API change(s) 💬 Discussion labels Mar 21, 2022
@scikit-image scikit-image unlocked this conversation Mar 21, 2022
@grlee77 grlee77 reopened this Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏩ type: Enhancement Improve existing features 📜 type: API Involves API change(s) 💬 Discussion
Projects
None yet
Development

No branches or pull requests

4 participants