Merge pull request #2500 from meeseeksmachine/auto-backport-of-pr-249…

…7-on-v3.7.x

Backport PR #2497 on branch v3.7.x (Deselect subset selection tool on deleting active subset)

CHANGES.rst

@@ -7,6 +7,9 @@ Bug Fixes
 - Fixed bug which did not update all references to a viewer's ID when
   updating a viewer's reference name. [#2479]
 
+- Deleting a subset while actively editing it now deselects the subset tool,
+  preventing the appearance of "ghost" subsets. [#2497]
+
 Cubeviz
 ^^^^^^^
 

jdaviz/configs/cubeviz/plugins/viewers.py

@@ -177,6 +177,7 @@ def _on_subset_delete(self, msg):
         # delete any ShadowSpatialSpectral mark for which either of the spectral or spatial marks
         # no longer exists by matching the uuid of the msg subset to the uuid of the subsets
         # in ShadowSpatialSpectral
+        super()._on_subset_delete(msg)
         self.figure.marks = [m for m in self.figure.marks
                              if not (isinstance(m, ShadowSpatialSpectral)
                                      and msg.subset.uuid in [m.spatial_uuid, m.spectral_uuid])]

jdaviz/configs/default/plugins/viewers.py

@@ -179,7 +179,8 @@ def _on_layers_update(self, layers=None):
                 self._expected_subset_layer_default(layer)
 
     def _on_subset_create(self, msg):
-        if self.__class__.__name__ == 'MosvizTableViewer':
+        from jdaviz.configs.mosviz.plugins.viewers import MosvizTableViewer
+        if isinstance(self, MosvizTableViewer):
             # MosvizTableViewer uses this as a mixin, but we do not need any of this layer
             # logic there
             return
@@ -191,6 +192,30 @@ def _on_subset_create(self, msg):
         if msg.subset.label not in self._expected_subset_layers and msg.subset.label:
             self._expected_subset_layers.append(msg.subset.label)
 
+    def _on_subset_delete(self, msg):
+        """
+        This is needed to remove the "ghost" subset left over when the subset tool is active,
+        and the active subset is deleted. https://github.com/spacetelescope/jdaviz/issues/2499
+        is open to revert/update this if it ends up being addressed upstream in
+        https://github.com/glue-viz/glue-jupyter/issues/401.
+        """
+        from jdaviz.configs.mosviz.plugins.viewers import MosvizTableViewer
+        if isinstance(self, MosvizTableViewer):
+            # MosvizTableViewer uses this as a mixin, but we do not need any of this layer
+            # logic there
+            return
+
+        subset_tools = ['bqplot:truecircle', 'bqplot:rectangle', 'bqplot:ellipse',
+                        'bqplot:circannulus', 'bqplot:xrange']
+
+        if not len(self.session.edit_subset_mode.edit_subset):
+            if self.toolbar.active_tool_id in subset_tools:
+                if (hasattr(self.toolbar, "default_tool_priority") and
+                        len(self.toolbar.default_tool_priority)):
+                    self.toolbar.active_tool_id = self.toolbar.default_tool_priority[0]
+                else:
+                    self.toolbar.active_tool = None
+
     @property
     def active_image_layer(self):
         """Active image layer in the viewer, if available."""

jdaviz/tests/test_subsets.py

@@ -237,6 +237,8 @@ def test_region_spectral_spatial(cubeviz_helper, spectral_cube_wcs):
     dc.remove_subset_group(dc.subset_groups[1])
 
     assert len([m for m in spectrum_viewer.figure.marks if isinstance(m, ShadowSpatialSpectral)]) == 0  # noqa
+    # Check that the subset selection tool was not deactivated by deleting inactive subset
+    assert spectrum_viewer.toolbar.active_tool_id == "bqplot:xrange"
 
     spectrum_viewer.session.edit_subset_mode._mode = NewMode
     flux_viewer.toolbar.active_tool = flux_viewer.toolbar.tools['bqplot:rectangle']
@@ -813,3 +815,29 @@ def test_multi_mask_subset(specviz_helper, spectrum1d):
     plugin.vue_freeze_subset()
     reg = specviz_helper.app.get_subsets()
     assert reg["Subset 1"][0]["region"] == 4
+
+
+def test_delete_subsets(cubeviz_helper, spectral_cube_wcs):
+    """
+    Test that the toolbar selections get reset when the subset being actively edited gets deleted.
+    """
+    data = Spectrum1D(flux=np.ones((128, 128, 256)) * u.nJy, wcs=spectral_cube_wcs)
+    cubeviz_helper.load_data(data, data_label="Test Flux")
+    dc = cubeviz_helper.app.data_collection
+
+    spectrum_viewer = cubeviz_helper.app.get_viewer("spectrum-viewer")
+    spectrum_viewer.toolbar.active_tool = spectrum_viewer.toolbar.tools['bqplot:xrange']
+    spectrum_viewer.apply_roi(XRangeROI(5, 15.5))
+
+    dc.remove_subset_group(dc.subset_groups[0])
+
+    assert spectrum_viewer.toolbar.active_tool_id == "jdaviz:selectslice"
+
+    flux_viewer = cubeviz_helper.app.get_viewer("flux-viewer")
+    # We set the active tool here to trigger a reset of the Subset state to "Create New"
+    flux_viewer.toolbar.active_tool = flux_viewer.toolbar.tools['bqplot:rectangle']
+    flux_viewer.apply_roi(RectangularROI(1, 3.5, -0.2, 3.3))
+
+    dc.remove_subset_group(dc.subset_groups[0])
+
+    assert flux_viewer.toolbar.active_tool is None