-
Notifications
You must be signed in to change notification settings - Fork 64
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
Fix: return object-typed properties as NumPy arrays in skimage.measure.regionprops_table
#272
Conversation
Can one of the admins verify this patch? |
@quasiben, could you please look at this one? |
add to allowlist |
I fixed a small formatting issue that was making tests fail, now it's all green besides label checker, but I can't assign those. |
@grlee77, could you please review this? 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @alxndrkalinin, the main fix here looks good to me!
I have one request to revert changes to the non-object code path in _regionprops.py
as I think it will be slower. If you measured an improvement, though, please let me know.
python/cucim/src/cucim/skimage/measure/tests/test_regionprops.py
Outdated
Show resolved
Hide resolved
python/cucim/src/cucim/skimage/measure/tests/test_regionprops.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Gregory Lee <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback @grlee77!
Please see 2 small suggestions I have – if you think they're unnecessary, I'll be happy to just revert back to the list for non-object props.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @alxndrkalinin, this looks good now.
This PR fixes adding properties whose results are objects to the dictionary that is returned by
skimage.measure.regionprops_table
.Minimal example reproducing the issue:
raises
TypeError: Implicit conversion to a NumPy array is not allowed.
, while returning correct values is expected (also see modified unit tests).For some reason, only
slice
property was checked for, although the list of such properties already exists inOBJECT_COLUMNS
.