-
Notifications
You must be signed in to change notification settings - Fork 70
Fix crash on unsigned integer input to cucim.skimage.morphology.reconstruction
#967
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
base: release/25.12
Are you sure you want to change the base?
Fix crash on unsigned integer input to cucim.skimage.morphology.reconstruction
#967
Conversation
ca39a5a to
c14eb25
Compare
Signed-off-by: Gregory R. Lee <[email protected]>
c14eb25 to
dbe8839
Compare
jakirkham
left a comment
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 Greg! 🙏
Generally this looks reasonable
Had a couple questions below
| # Rescale image intensity so that we can see dim features. | ||
| image = rescale_intensity(image, in_range=(50, 200)) | ||
|
|
||
| seed = cp.copy(image) |
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.
seed could be confused with random number generator seed. Should we use a different name like image2 or similar?
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.
sure, updated as suggested
| mask = image | ||
|
|
||
| seed = seed.astype(unsigned_dtype) | ||
| filled = reconstruction(seed, mask, method="erosion") |
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.
Maybe this could be image2_eroded?
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.
updated
resolves #964
This
reconstructionfunction is a hybrid implementation that still relies on an inner Cythonreconstruction_loopfunction from upstream scikit-image. I found that upgrading the input as done here avoids the observed crash and gives output that matches the scikit-image one.