Prevent destruction of CircularAnnulusROI by resizing below inner_radius
#383
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #383 +/- ##
==========================================
+ Coverage 86.91% 87.35% +0.43%
==========================================
Files 89 89
Lines 4990 5004 +14
==========================================
+ Hits 4337 4371 +34
+ Misses 653 633 -20
☔ View full report in Codecov by Sentry. |
| tool.interact.selected = [(150.0, 275.0), (150.0, 325.0)] | ||
| tool.interact.brushing = False | ||
| assert_allclose(roi.xc, 151.00) | ||
| # assert_allclose(roi.yc, 326.75) # trying to use `move`, but this is not how it works... |
There was a problem hiding this comment.
I don't understand this comment. Is there a bug that need fixing here?
There was a problem hiding this comment.
The "bug" is me not knowing how to work with tool.interact to reproduce the interaction of clicking inside the subset, then dragging it 50 pixels upwards and release it (i.e. do a move of +50 pixels in y). Just left here as a reminder, but if I don't find a solution, the commented part can be removed again.
dhomeier
force-pushed
the
save-circular-annulus
branch
from
0ae033c to
2c2aa86
Compare
There was a problem hiding this comment.
Seems reasonable to me, thanks! But as noted in #376 (comment) I don't think we should be adding more default tools for now.
|
Is there a way to test stuff without having to add things to default tools proper? |
Description
#376 (comment) brought to light an issue with the little-documented resize functionality of the bqplot brush tool when modifying
BqplotCircularAnnulusMode: since the update from the modifiedself._roionly changes the outer and not the inner radius, it is possible to decrease the former below an invalid valueouter_radius <= inner_radius, as a result destroying the subregion. This PR prevents this by keepinginner_radiusjust belowouter_radius.Such a minimal-width circular annulus will typically become impossible to manipulate interactively, but it still exists and can e.g. be restored to some sensible inner radius via the menu.
I have added a basic test for creating this subset type, but was unable to emulate even the
moveoperation this way, let alone resizing – pointers how to do this withtool.interactwelcome.