Merge pull request #383 from dhomeier/save-circular-annulus

Prevent destruction of `CircularAnnulusROI` by resizing below `inner_radius`

glue_jupyter/bqplot/common/tools.py

@@ -441,7 +441,8 @@ def update_selection(self, *args):
                 if self._roi is None:
                     inner_r = outer_r * 0.5  # Hardcoded for now, user can edit later.
                 else:
-                    inner_r = self._roi.inner_radius
+                    # Resizing only changes outer r, avoid having inner_r >= outer_r afterwards.
+                    inner_r = min(self._roi.inner_radius, outer_r * 0.999999)
 
                 roi = CircularAnnulusROI(xc=xc, yc=yc, inner_radius=inner_r, outer_radius=outer_r)
 

glue_jupyter/bqplot/image/viewer.py

@@ -30,8 +30,8 @@ class BqplotImageView(BqplotBaseView):
     _state_cls = BqplotImageViewerState
     _options_cls = ImageViewerStateWidget
 
-    tools = ['bqplot:home', 'bqplot:panzoom', 'bqplot:rectangle', 'bqplot:circle', 'bqplot:polygon',
-             'bqplot:ellipse', 'bqplot:truecircle']
+    tools = ['bqplot:home', 'bqplot:panzoom', 'bqplot:rectangle', 'bqplot:circle',
+             'bqplot:ellipse', 'bqplot:polygon']
 
     def __init__(self, session):
 

glue_jupyter/bqplot/tests/test_bqplot.py

@@ -3,8 +3,9 @@
 import numpy as np
 from numpy.testing import assert_allclose
 from nbconvert.preprocessors import ExecutePreprocessor
+from glue.config import viewer_tool
 from glue.core import Data
-from glue.core.roi import EllipticalROI
+from glue.core.roi import CircularAnnulusROI, EllipticalROI
 from ..common.tools import TrueCircularROI
 
 DATA = os.path.join(os.path.dirname(__file__), 'data')
@@ -271,12 +272,33 @@ def test_imshow_circular_brush(app, data_image):
     assert_allclose(roi.radius_y, 273.25)
 
 
+def test_imshow_elliptical_brush(app, data_image):
+    v = app.imshow(data=data_image)
+    v.state.aspect = 'auto'
+
+    tool = v.toolbar.tools['bqplot:ellipse']
+    tool.activate()
+    tool.interact.brushing = True
+    tool.interact.selected = [(1.5, 3.5), (300.5, 550)]
+    tool.interact.brushing = False
+
+    roi = data_image.subsets[0].subset_state.roi
+    assert isinstance(roi, EllipticalROI)
+    assert_allclose(roi.xc, 151.00)
+    assert_allclose(roi.yc, 276.75)
+
+
+# Tools that are not part of the default set of BqplotImageView; manually added for testing
 def test_imshow_true_circular_brush(app, data_image):
 
     v = app.imshow(data=data_image)
     v.state.aspect = 'auto'
 
-    tool = v.toolbar.tools['bqplot:truecircle']
+    tool_id = 'bqplot:truecircle'
+    mode_cls = viewer_tool.members[tool_id]
+    v.toolbar.add_tool(mode_cls(v))
+
+    tool = v.toolbar.tools[tool_id]
     tool.activate()
     tool.interact.brushing = True
     tool.interact.selected = [(1.5, 3.5), (300.5, 550)]
@@ -289,20 +311,30 @@ def test_imshow_true_circular_brush(app, data_image):
     assert_allclose(roi.radius, 220.2451)
 
 
-def test_imshow_elliptical_brush(app, data_image):
+def test_imshow_circular_annulus_brush(app, data_image):
+
     v = app.imshow(data=data_image)
     v.state.aspect = 'auto'
 
-    tool = v.toolbar.tools['bqplot:ellipse']
+    tool_id = 'bqplot:circannulus'
+    mode_cls = viewer_tool.members[tool_id]
+    v.toolbar.add_tool(mode_cls(v))
+
+    tool = v.toolbar.tools[tool_id]
     tool.activate()
     tool.interact.brushing = True
     tool.interact.selected = [(1.5, 3.5), (300.5, 550)]
     tool.interact.brushing = False
 
     roi = data_image.subsets[0].subset_state.roi
-    assert isinstance(roi, EllipticalROI)
+    assert isinstance(roi, CircularAnnulusROI)
     assert_allclose(roi.xc, 151.00)
     assert_allclose(roi.yc, 276.75)
+    assert_allclose(roi.outer_radius, 211.375)
+    assert_allclose(roi.inner_radius, 105.6875)
+
+    # should try to test `move` and `resize` as well, but this probably
+    # needs to go through `update_selection` directly
 
 
 def test_imshow_equal_aspect(app, data_image):