Allow enforcing persistenly circular ROI in draw tool

glue_jupyter/bqplot/common/tools.py

@@ -27,6 +27,10 @@
 ICONS_DIR = os.path.join(os.path.dirname(__file__), '..', '..', 'icons')
 
 
+class TrueCircularROI(CircularROI):
+    pass
+
+
 class InteractCheckableTool(CheckableTool):
 
     def __init__(self, viewer):
@@ -275,7 +279,7 @@ class BqplotCircleMode(BqplotSelectionTool):
     action_text = 'Circular ROI'
     tool_tip = 'Define a circular region of interest'
 
-    def __init__(self, viewer, roi=None, finalize_callback=None, **kwargs):
+    def __init__(self, viewer, roi=None, finalize_callback=None, strict_circle=None, **kwargs):
 
         super().__init__(viewer, **kwargs)
 
@@ -291,6 +295,7 @@ def __init__(self, viewer, roi=None, finalize_callback=None, **kwargs):
         border_style['stroke'] = INTERACT_COLOR
         self.interact.style = style
         self.interact.border_style = border_style
+        self._strict_circle = strict_circle
 
         if roi is not None:
             self.update_from_roi(roi)
@@ -309,17 +314,19 @@ def update_selection(self, *args):
                 y = self.interact.selected_y
                 # similar to https://github.com/glue-viz/glue/blob/b14ccffac6a5
                 # 271c2869ead9a562a2e66232e397/glue/core/roi.py#L1275-L1297
-                # We should now check if the radius in data coordinates is the same
-                # along x and y, as if so then we can return a circle, otherwise we
-                # should return an ellipse.
+                # If _strict_circle set, enforce returning a circle; otherwise check
+                # if the radius in data coordinates is (nearly) the same along x and y,
+                # to return a circle as well, else we should return an ellipse.
                 xc = x.mean()
                 yc = y.mean()
                 rx = abs(x[1] - x[0])/2
                 ry = abs(y[1] - y[0])/2
                 # We use a tolerance of 1e-2 below to match the tolerance set in glue-core
                 # https://github.com/glue-viz/glue/blob/6b968b352bc5ad68b95ad5e3bb25550782a69ee8/glue/viewers/matplotlib/state.py#L198
-                if np.allclose(rx, ry, rtol=1e-2):
-                    roi = CircularROI(xc=xc, yc=yc, radius=rx)
+                if self._strict_circle:
+                    roi = TrueCircularROI(xc=xc, yc=yc, radius=np.sqrt((rx**2 + ry**2) * 0.5))
+                elif np.allclose(rx, ry, rtol=1e-2):
+                    roi = CircularROI(xc=xc, yc=yc, radius=(rx + ry) * 0.5)
                 else:
                     roi = EllipticalROI(xc=xc, yc=yc, radius_x=rx, radius_y=ry)
                 self.viewer.apply_roi(roi)
@@ -329,8 +336,13 @@ def update_selection(self, *args):
     def update_from_roi(self, roi):
         if isinstance(roi, CircularROI):
             rx = ry = roi.radius
+            if isinstance(roi, TrueCircularROI) and self._strict_circle is not False:
+                self._strict_circle = True
         elif isinstance(roi, EllipticalROI):
-            rx, ry = roi.radius_x, roi.radius_y
+            if self._strict_circle:
+                rx, ry = np.sqrt((roi.radius_x ** 2 + roi.radius_y ** 2) * 0.5)
+            else:
+                rx, ry = roi.radius_x, roi.radius_y
         else:
             raise TypeError(f'Cannot initialize a BqplotCircleMode from a {type(roi)}')
         self.interact.selected_x = [roi.xc - rx, roi.xc + rx]
@@ -348,6 +360,21 @@ def activate(self):
         super().activate()
 
 
+@viewer_tool
+class BqplotTrueCircleMode(BqplotCircleMode):
+
+    tool_id = 'bqplot:truecircle'
+    tool_tip = 'Define a strictly circular region of interest'
+
+    def __init__(self, viewer, roi=None, finalize_callback=None, **kwargs):
+
+        super().__init__(viewer, **kwargs)
+
+        if kwargs.pop('strict_circle', True) is False:
+            raise ValueError('BqplotTrueCircleMode cannot have strict_circle=False')
+        self._strict_circle = True
+
+
 @viewer_tool
 class BqplotEllipseMode(BqplotCircleMode):
 
@@ -582,7 +609,10 @@ def press(self, x, y):
             if layer.visible and isinstance(subset_state, RoiSubsetState):
                 roi = subset_state.roi
                 if roi.defined() and roi.contains(x, y):
-                    if isinstance(roi, (EllipticalROI, CircularROI)):
+                    if isinstance(roi, EllipticalROI):
+                        self._active_tool = BqplotEllipseMode(
+                            self.viewer, roi=roi, finalize_callback=self.release)
+                    elif isinstance(roi, CircularROI):
                         self._active_tool = BqplotCircleMode(
                             self.viewer, roi=roi, finalize_callback=self.release)
                     elif isinstance(roi, (PolygonalROI, RectangularROI)):

glue_jupyter/bqplot/image/viewer.py

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

glue_jupyter/bqplot/tests/test_bqplot.py

@@ -5,6 +5,7 @@
 from nbconvert.preprocessors import ExecutePreprocessor
 from glue.core import Data
 from glue.core.roi import EllipticalROI
+from ..common.tools import TrueCircularROI
 
 DATA = os.path.join(os.path.dirname(__file__), 'data')
 
@@ -270,6 +271,40 @@ def test_imshow_circular_brush(app, data_image):
     assert_allclose(roi.radius_y, 273.25)
 
 
+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.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, TrueCircularROI)
+    assert_allclose(roi.xc, 151.00)
+    assert_allclose(roi.yc, 276.75)
+    assert_allclose(roi.radius, 220.2451)
+
+
+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)
+
+
 def test_imshow_equal_aspect(app, data_image):
     data = Data(array=np.random.random((100, 5)))
     app.data_collection.append(data)