Merge pull request #357 from pllim/exorcise-evil-twin

Fix duplicate xrange region bug when operators are used on existing regions

glue_jupyter/bqplot/common/tools.py

@@ -354,22 +354,43 @@ class BqplotXRangeMode(BqplotSelectionTool):
     action_text = 'X range ROI'
     tool_tip = 'Select a range of x values'
 
-    def __init__(self, viewer, **kwargs):
+    def __init__(self, viewer, roi=None, finalize_callback=None, **kwargs):
 
         super().__init__(viewer, **kwargs)
 
         self.interact = BrushIntervalSelector(scale=self.viewer.scale_x,
                                               color=INTERACT_COLOR)
 
+        if roi is not None:
+            self.update_from_roi(roi)
+
         self.interact.observe(self.update_selection, "brushing")
+        self.interact.observe(self.on_selection_change, "selected")
+        self.finalize_callback = finalize_callback
 
     def update_selection(self, *args):
+        if self.interact.brushing:
+            return
         with self.viewer._output_widget or nullcontext():
             if self.interact.selected is not None:
                 x = self.interact.selected
                 if x is not None and len(x):
                     roi = RangeROI(min=min(x), max=max(x), orientation='x')
                     self.viewer.apply_roi(roi)
+                    if self.finalize_callback is not None:
+                        self.finalize_callback()
+
+    def update_from_roi(self, roi):
+        with self.viewer._output_widget or nullcontext():
+            if isinstance(roi, RangeROI):
+                self.interact.selected = [roi.min, roi.max]
+            else:
+                raise TypeError(f'Cannot initialize a BqplotXRangeMode from a {type(roi)}')
+
+    def on_selection_change(self, *args):
+        if self.interact.selected is None:
+            if self.finalize_callback is not None:
+                self.finalize_callback()
 
     def activate(self):
         with self.viewer._output_widget or nullcontext():
@@ -385,23 +406,44 @@ class BqplotYRangeMode(BqplotSelectionTool):
     action_text = 'Y range ROI'
     tool_tip = 'Select a range of y values'
 
-    def __init__(self, viewer, **kwargs):
+    def __init__(self, viewer, roi=None, finalize_callback=None, **kwargs):
 
         super().__init__(viewer, **kwargs)
 
         self.interact = BrushIntervalSelector(scale=self.viewer.scale_y,
                                               orientation='vertical',
                                               color=INTERACT_COLOR)
 
+        if roi is not None:
+            self.update_from_roi(roi)
+
         self.interact.observe(self.update_selection, "brushing")
+        self.interact.observe(self.on_selection_change, "selected")
+        self.finalize_callback = finalize_callback
 
     def update_selection(self, *args):
+        if self.interact.brushing:
+            return
         with self.viewer._output_widget or nullcontext():
             if self.interact.selected is not None:
                 y = self.interact.selected
                 if y is not None and len(y):
                     roi = RangeROI(min=min(y), max=max(y), orientation='y')
                     self.viewer.apply_roi(roi)
+                    if self.finalize_callback is not None:
+                        self.finalize_callback()
+
+    def update_from_roi(self, roi):
+        with self.viewer._output_widget or nullcontext():
+            if isinstance(roi, RangeROI):
+                self.interact.selected = [roi.min, roi.max]
+            else:
+                raise TypeError(f'Cannot initialize a BqplotYRangeMode from a {type(roi)}')
+
+    def on_selection_change(self, *args):
+        if self.interact.selected is None:
+            if self.finalize_callback is not None:
+                self.finalize_callback()
 
     def activate(self):
         with self.viewer._output_widget or nullcontext():

glue_jupyter/bqplot/profile/tests/test_viewer.py

@@ -1,12 +1,15 @@
 from astropy.wcs import WCS
 
 import numpy as np
+import pytest
 from numpy.testing import assert_allclose, assert_equal
 
 from astropy import units as u
 
 from glue.core import Data
+from glue.core.edit_subset_mode import AndNotMode
 from glue.core.roi import XRangeROI
+from glue.core.subset import AndState, InvertState, RangeSubsetState
 from glue.config import unit_converter, settings
 from glue.plugins.wcs_autolinking.wcs_autolinking import WCSLink
 
@@ -61,6 +64,7 @@ def to_unit(self, data, cid, values, original_units, target_units):
                                                           equivalencies=u.spectral())
 
 
+@pytest.mark.filterwarnings("ignore:No observer defined on WCS.*")
 def test_unit_conversion(app):
 
     settings.UNIT_CONVERTER = 'test-spectral'
@@ -157,3 +161,80 @@ def test_unit_conversion(app):
 
     assert len(d2.subsets) == 1
     assert_equal(d2.subsets[0].to_mask(), [0, 1, 1])
+
+
+def test_composite_xrange(app):
+    session = app.session
+
+    wcs2 = WCS(naxis=1)
+    wcs2.wcs.ctype = ['WAVE']
+    wcs2.wcs.crval = [10]
+    wcs2.wcs.cdelt = [10]
+    wcs2.wcs.crpix = [1]
+    wcs2.wcs.cunit = ['cm']
+
+    d2 = Data(f2=[2000, 1000, 3000])
+    d2.get_component('f2').units = 'mJy'
+    d2.coords = wcs2
+
+    data_collection = session.data_collection
+    data_collection.append(d2)
+
+    viewer = app.profile1d(data=d2)
+    tool = viewer.toolbar.tools['bqplot:xrange']
+    tool.activate()
+    tool.interact.brushing = True
+    tool.interact.selected = [15, 35]
+    tool.interact.brushing = False
+
+    session.edit_subset_mode.mode = AndNotMode
+
+    tool.interact.brushing = True
+    tool.interact.selected = [25, 45]
+    tool.interact.brushing = False
+    tool.deactivate()
+
+    sbst = session.data_collection.subset_groups[0].subset_state
+    assert isinstance(sbst, AndState)
+    assert isinstance(sbst.state1, RangeSubsetState)
+    assert (isinstance(sbst.state2, InvertState) and
+            isinstance(sbst.state2.state1, RangeSubsetState))
+
+
+def test_composite_yrange(app):
+    session = app.session
+
+    wcs2 = WCS(naxis=1)
+    wcs2.wcs.ctype = ['WAVE']
+    wcs2.wcs.crval = [10]
+    wcs2.wcs.cdelt = [10]
+    wcs2.wcs.crpix = [1]
+    wcs2.wcs.cunit = ['cm']
+
+    d2 = Data(f2=[2000, 1000, 3000])
+    d2.get_component('f2').units = 'mJy'
+    d2.coords = wcs2
+
+    data_collection = session.data_collection
+    data_collection.append(d2)
+
+    viewer = app.profile1d(data=d2)
+    tool = viewer.toolbar.tools['bqplot:yrange']
+
+    tool.activate()
+    tool.interact.brushing = True
+    tool.interact.selected = [1100, 1500]
+    tool.interact.brushing = False
+
+    session.edit_subset_mode.mode = AndNotMode
+
+    tool.interact.brushing = True
+    tool.interact.selected = [1450, 1550]
+    tool.interact.brushing = False
+    tool.deactivate()
+
+    sbst = session.data_collection.subset_groups[0].subset_state
+    assert isinstance(sbst, AndState)
+    assert isinstance(sbst.state1, RangeSubsetState)
+    assert (isinstance(sbst.state2, InvertState) and
+            isinstance(sbst.state2.state1, RangeSubsetState))

glue_jupyter/bqplot/profile/viewer.py

@@ -28,7 +28,7 @@ class BqplotProfileView(BqplotBaseView):
     _layer_style_widget_cls = ProfileLayerStateWidget
 
     tools = ['bqplot:home', 'bqplot:panzoom', 'bqplot:panzoom_x', 'bqplot:panzoom_y',
-             'bqplot:xrange']
+             'bqplot:xrange', 'bqplot:yrange']
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)