FEAT: Annulus draw tool for Imviz (#2240)

* FEAT: Annulus draw tool for Imviz. TODO: Need to fix icon. TST: Add tests. [ci skip] [rtd skip] * Pull in #2204 into this PR and bump upstream pins * Changed function name upstream in glue-astronomy during review process * Proper annulus icon from J. Kotler Co-authored-by: Jennifer Kotler <[email protected]> * Avoid error traceback with bad annulus radii * Disable recentering for annulus * Fix test failure because not sure why spectral region is using Imviz centering method but okay. --------- Co-authored-by: Jennifer Kotler <[email protected]> Co-authored-by: Ricky O'Steen <[email protected]>
spacetelescope · Jun 29, 2023 · bc6cdee · bc6cdee
1 parent 40116b9
commit bc6cdee
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 75 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -20,7 +20,8 @@ Imviz
 - Added the ability to load DS9 region files (``.reg``) using the ``IMPORT DATA``
   button. However, this only works after loading at least one image into Imviz. [#2201]
 
-- Added support for new ``CircularAnnulusROI`` subset from glue. [#2201]
+- Added support for new ``CircularAnnulusROI`` subset from glue, including
+  a new draw tool. [#2201, #2240]
 
 Mosviz
 ^^^^^^

diff --git a/jdaviz/app.py b/jdaviz/app.py
@@ -12,11 +12,8 @@
 from astropy import units as u
 from astropy.nddata import CCDData, NDData
 from astropy.io import fits
-from astropy.coordinates import Angle
 from astropy.time import Time
 from astropy.utils.decorators import deprecated
-from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion, EllipsePixelRegion
-
 from echo import CallbackProperty, DictCallbackProperty, ListCallbackProperty
 from ipygoldenlayout import GoldenLayout
 from ipysplitpanes import SplitPanes
@@ -39,9 +36,9 @@
 from glue.core.state_objects import State
 from glue.core.subset import (Subset, RangeSubsetState, RoiSubsetState,
                               CompositeSubsetState, InvertState)
-from glue.core.roi import CircularROI, EllipticalROI, RectangularROI
 from glue.core.units import unit_converter
 from glue_astronomy.spectral_coordinates import SpectralCoordinates
+from glue_astronomy.translators.regions import roi_subset_state_to_region
 from glue_jupyter.app import JupyterApplication
 from glue_jupyter.common.toolbar_vuetify import read_icon
 from glue_jupyter.state_traitlets_helpers import GlueState
@@ -1055,27 +1052,8 @@ def _get_range_subset_bounds(self, subset_state,
         return spec_region
 
     def _get_roi_subset_definition(self, subset_state):
-        _around_decimals = 6
-        roi = subset_state.roi
-        roi_as_region = None
-        if isinstance(roi, CircularROI):
-            x, y = roi.get_center()
-            r = roi.radius
-            roi_as_region = CirclePixelRegion(PixCoord(x, y), r)
-
-        elif isinstance(roi, RectangularROI):
-            theta = np.around(np.degrees(roi.theta), decimals=_around_decimals)
-            roi_as_region = RectanglePixelRegion(PixCoord(roi.center()[0], roi.center()[1]),
-                                                 roi.width(), roi.height(), Angle(theta, "deg"))
-
-        elif isinstance(roi, EllipticalROI):
-            xc = roi.xc
-            yc = roi.yc
-            rx = roi.radius_x
-            ry = roi.radius_y
-            theta = np.around(np.degrees(roi.theta), decimals=_around_decimals)
-            roi_as_region = EllipsePixelRegion(PixCoord(xc, yc), rx * 2, ry * 2, Angle(theta, "deg"))  # noqa: E501
-
+        # TODO: Imviz: Return sky region if link type is WCS.
+        roi_as_region = roi_subset_state_to_region(subset_state)
         return [{"name": subset_state.roi.__class__.__name__,
                  "glue_state": subset_state.__class__.__name__,
                  "region": roi_as_region,

diff --git a/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py b/jdaviz/configs/default/plugins/subset_plugin/subset_plugin.py
@@ -7,8 +7,8 @@
 from glue.core.message import EditSubsetMessage, SubsetUpdateMessage
 from glue.core.edit_subset_mode import (AndMode, AndNotMode, OrMode,
                                         ReplaceMode, XorMode)
-from glue.core.roi import CircularROI, EllipticalROI, RectangularROI
-from glue.core.subset import RoiSubsetState, RangeSubsetState, CompositeSubsetState
+from glue.core.roi import CircularROI, CircularAnnulusROI, EllipticalROI, RectangularROI
+from glue.core.subset import RoiSubsetState, RangeSubsetState
 from glue.icons import icon_path
 from glue_jupyter.widgets.subset_mode_vuetify import SelectionModeMenu
 from glue_jupyter.common.toolbar_vuetify import read_icon
@@ -149,7 +149,11 @@ def _unpack_get_subsets_for_ui(self):
         _around_decimals = 6  # Avoid 30 degrees from coming back as 29.999999999999996
         if not subset_information:
             return
-        if len(subset_information) == 1:
+        if ((len(subset_information) == 1) and
+                (isinstance(subset_information[0]["subset_state"], RangeSubsetState) or
+                 (isinstance(subset_information[0]["subset_state"], RoiSubsetState) and
+                  isinstance(subset_information[0]["subset_state"].roi,
+                             (CircularROI, RectangularROI, EllipticalROI))))):
             self.is_centerable = True
         else:
             self.is_centerable = False
@@ -161,7 +165,7 @@ def _unpack_get_subsets_for_ui(self):
             glue_state = spec["glue_state"]
             if isinstance(subset_state, RoiSubsetState):
                 if isinstance(subset_state.roi, CircularROI):
-                    x, y = subset_state.roi.get_center()
+                    x, y = subset_state.roi.center()
                     r = subset_state.roi.radius
                     subset_definition = [{"name": "X Center", "att": "xc", "value": x, "orig": x},
                                          {"name": "Y Center", "att": "yc", "value": y, "orig": y},
@@ -178,8 +182,7 @@ def _unpack_get_subsets_for_ui(self):
                         {"name": "Angle", "att": "theta", "value": theta, "orig": theta})
 
                 elif isinstance(subset_state.roi, EllipticalROI):
-                    xc = subset_state.roi.xc
-                    yc = subset_state.roi.yc
+                    xc, yc = subset_state.roi.center()
                     rx = subset_state.roi.radius_x
                     ry = subset_state.roi.radius_y
                     theta = np.around(np.degrees(subset_state.roi.theta), decimals=_around_decimals)
@@ -190,6 +193,17 @@ def _unpack_get_subsets_for_ui(self):
                         {"name": "Y Radius", "att": "radius_y", "value": ry, "orig": ry},
                         {"name": "Angle", "att": "theta", "value": theta, "orig": theta}]
 
+                elif isinstance(subset_state.roi, CircularAnnulusROI):
+                    x, y = subset_state.roi.center()
+                    inner_r = subset_state.roi.inner_radius
+                    outer_r = subset_state.roi.outer_radius
+                    subset_definition = [{"name": "X Center", "att": "xc", "value": x, "orig": x},
+                                         {"name": "Y Center", "att": "yc", "value": y, "orig": y},
+                                         {"name": "Inner radius", "att": "inner_radius",
+                                          "value": inner_r, "orig": inner_r},
+                                         {"name": "Outer radius", "att": "outer_radius",
+                                          "value": outer_r, "orig": outer_r}]
+
                 subset_type = subset_state.roi.__class__.__name__
 
             elif isinstance(subset_state, RangeSubsetState):
@@ -303,6 +317,7 @@ def _check_input(self):
         reason = ""
         for index, sub in enumerate(self.subset_definitions):
             lo = hi = xmin = xmax = ymin = ymax = None
+            inner_radius = outer_radius = None
             for d_att in sub:
                 if d_att["att"] == "lo":
                     lo = d_att["value"]
@@ -320,6 +335,10 @@ def _check_input(self):
                     ymin = d_att["value"]
                 elif d_att["att"] == "ymax":
                     ymax = d_att["value"]
+                elif d_att["att"] == "outer_radius":
+                    outer_radius = d_att["value"]
+                elif d_att["att"] == "inner_radius":
+                    inner_radius = d_att["value"]
 
                 if lo and hi and hi <= lo:
                     status = False
@@ -329,6 +348,10 @@ def _check_input(self):
                     status = False
                     reason = "Failed to update Subset: width and length must be positive scalars"
                     break
+                elif inner_radius and outer_radius and inner_radius >= outer_radius:
+                    status = False
+                    reason = "Failed to update Subset: inner radius must be less than outer radius"
+                    break
 
         return status, reason
 
@@ -375,39 +398,13 @@ def get_center(self):
             depending on the Subset type, if applicable.
             If Subset is not centerable, this returns `None`.
 
-        Raises
-        ------
-        NotImplementedError
-            Subset type is not supported.
-
         """
         # Composite region cannot be centered.
         if not self.is_centerable:  # no-op
             return
 
         subset_state = self.subset_select.selected_subset_state
-
-        if isinstance(subset_state, RoiSubsetState):
-            sbst_obj = subset_state.roi
-            if isinstance(sbst_obj, (CircularROI, EllipticalROI)):
-                cen = sbst_obj.get_center()
-            elif isinstance(sbst_obj, RectangularROI):
-                cen = sbst_obj.center()
-            else:  # pragma: no cover
-                raise NotImplementedError(
-                    f'Getting center of {sbst_obj.__class__} is not supported')
-
-        elif isinstance(subset_state, RangeSubsetState):
-            cen = (subset_state.hi - subset_state.lo) * 0.5 + subset_state.lo
-
-        elif isinstance(subset_state, CompositeSubsetState):
-            cen = None
-
-        else:  # pragma: no cover
-            raise NotImplementedError(
-                f'Getting center of {subset_state.__class__} is not supported')
-
-        return cen
+        return subset_state.center()
 
     def set_center(self, new_cen, update=False):
         """Set the desired center for the selected Subset, if applicable.
@@ -439,7 +436,7 @@ def set_center(self, new_cen, update=False):
         if isinstance(subset_state, RoiSubsetState):
             x, y = new_cen
             sbst_obj = subset_state.roi
-            if isinstance(sbst_obj, (CircularROI, EllipticalROI)):
+            if isinstance(sbst_obj, (CircularROI, CircularAnnulusROI, EllipticalROI)):
                 self._set_value_in_subset_definition(0, "X Center", "value", x)
                 self._set_value_in_subset_definition(0, "Y Center", "value", y)
             elif isinstance(sbst_obj, RectangularROI):
@@ -454,7 +451,7 @@ def set_center(self, new_cen, update=False):
                 raise NotImplementedError(f'Recentering of {sbst_obj.__class__} is not supported')
 
         elif isinstance(subset_state, RangeSubsetState):
-            dx = new_cen - ((subset_state.hi - subset_state.lo) * 0.5 + subset_state.lo)
+            dx = new_cen - subset_state.center()
             self._set_value_in_subset_definition(0, "Lower bound", "value", subset_state.lo + dx)
             self._set_value_in_subset_definition(0, "Upper bound", "value", subset_state.hi + dx)
 

diff --git a/jdaviz/configs/imviz/plugins/viewers.py b/jdaviz/configs/imviz/plugins/viewers.py
@@ -24,7 +24,7 @@ class ImvizImageView(JdavizViewerMixin, BqplotImageView, AstrowidgetsImageViewer
                     ['jdaviz:homezoom', 'jdaviz:prevzoom'],
                     ['jdaviz:boxzoommatch', 'jdaviz:boxzoom'],
                     ['jdaviz:panzoommatch', 'jdaviz:imagepanzoom'],
-                    ['bqplot:circle', 'bqplot:rectangle', 'bqplot:ellipse',
+                    ['bqplot:circle', 'bqplot:rectangle', 'bqplot:ellipse', 'bqplot:circannulus',
                      'jdaviz:singlepixelregion'],
                     ['jdaviz:blinkonce', 'jdaviz:contrastbias'],
                     ['jdaviz:sidebar_plot', 'jdaviz:sidebar_export', 'jdaviz:sidebar_compass']

diff --git a/jdaviz/core/tools.py b/jdaviz/core/tools.py
@@ -9,8 +9,9 @@
                                               HomeTool, BqplotPanZoomMode,
                                               BqplotPanZoomXMode, BqplotPanZoomYMode,
                                               BqplotRectangleMode, BqplotCircleMode,
-                                              BqplotEllipseMode, BqplotXRangeMode,
-                                              BqplotYRangeMode, BqplotSelectionTool,
+                                              BqplotEllipseMode, BqplotCircularAnnulusMode,
+                                              BqplotXRangeMode, BqplotYRangeMode,
+                                              BqplotSelectionTool,
                                               INTERACT_COLOR)
 from bqplot.interacts import BrushSelector, BrushIntervalSelector
 
@@ -25,6 +26,7 @@
 BqplotRectangleMode.icon = os.path.join(ICON_DIR, 'select_xy.svg')
 BqplotCircleMode.icon = os.path.join(ICON_DIR, 'select_circle.svg')
 BqplotEllipseMode.icon = os.path.join(ICON_DIR, 'select_ellipse.svg')
+BqplotCircularAnnulusMode.icon = os.path.join(ICON_DIR, 'select_annulus.svg')
 BqplotXRangeMode.icon = os.path.join(ICON_DIR, 'select_x.svg')
 BqplotYRangeMode.icon = os.path.join(ICON_DIR, 'select_y.svg')
 

diff --git a/jdaviz/data/icons/select_annulus.svg b/jdaviz/data/icons/select_annulus.svg
diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py
@@ -3,9 +3,10 @@
 from astropy import units as u
 from astropy.tests.helper import assert_quantity_allclose
 from glue.core import Data
-from glue.core.roi import CircularROI, EllipticalROI, RectangularROI, XRangeROI
+from glue.core.roi import CircularROI, CircularAnnulusROI, EllipticalROI, RectangularROI, XRangeROI
 from glue.core.edit_subset_mode import AndMode, AndNotMode, OrMode, XorMode
-from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion, EllipsePixelRegion
+from regions import (PixCoord, CirclePixelRegion, RectanglePixelRegion, EllipsePixelRegion,
+                     CircleAnnulusPixelRegion)
 from numpy.testing import assert_allclose
 from specutils import SpectralRegion, Spectrum1D
 
@@ -316,7 +317,7 @@ def test_composite_region_from_subset_3d(cubeviz_helper):
                        'subset_state': reg[-1]['subset_state']}
 
     cubeviz_helper.app.session.edit_subset_mode.mode = OrMode
-    viewer.apply_roi(EllipticalROI(30, 30, 3, 6))
+    viewer.apply_roi(EllipticalROI(xc=30, yc=30, radius_x=3, radius_y=6))
     reg = cubeviz_helper.app.get_subsets("Subset 1")
     ellipse1 = EllipsePixelRegion(center=PixCoord(x=30, y=30),
                                   width=6, height=12, angle=0.0 * u.deg)
@@ -368,7 +369,7 @@ def test_composite_region_with_consecutive_and_not_states(cubeviz_helper):
                        'subset_state': reg[-1]['subset_state']}
 
     cubeviz_helper.app.session.edit_subset_mode.mode = AndNotMode
-    viewer.apply_roi(EllipticalROI(30, 30, 3, 6))
+    viewer.apply_roi(EllipticalROI(xc=30, yc=30, radius_x=3, radius_y=6))
     reg = cubeviz_helper.app.get_subsets("Subset 1")
     ellipse1 = EllipsePixelRegion(center=PixCoord(x=30, y=30),
                                   width=6, height=12, angle=0.0 * u.deg)
@@ -413,7 +414,7 @@ def test_composite_region_with_imviz(imviz_helper, image_2d_wcs):
     arr = np.ones((10, 10))
 
     data_label = 'image-data'
-    viewer = imviz_helper.app.get_viewer('imviz-0')
+    viewer = imviz_helper.default_viewer
     imviz_helper.load_data(arr, data_label=data_label, show_in_viewer=True)
     viewer.apply_roi(CircularROI(xc=5, yc=5, radius=2))
     reg = imviz_helper.app.get_subsets("Subset 1")
@@ -422,25 +423,33 @@ def test_composite_region_with_imviz(imviz_helper, image_2d_wcs):
                        'subset_state': reg[-1]['subset_state']}
 
     imviz_helper.app.session.edit_subset_mode.mode = AndNotMode
-    viewer.apply_roi(RectangularROI(2, 4, 2, 4))
+    viewer.apply_roi(RectangularROI(xmin=2, xmax=4, ymin=2, ymax=4))
     reg = imviz_helper.app.get_subsets("Subset 1")
     rectangle1 = RectanglePixelRegion(center=PixCoord(x=3, y=3),
                                       width=2, height=2, angle=0.0 * u.deg)
     assert reg[-1] == {'name': 'RectangularROI', 'glue_state': 'AndNotState', 'region': rectangle1,
                        'subset_state': reg[-1]['subset_state']}
 
     imviz_helper.app.session.edit_subset_mode.mode = AndNotMode
-    viewer.apply_roi(EllipticalROI(3, 3, 3, 6))
+    viewer.apply_roi(EllipticalROI(xc=3, yc=3, radius_x=3, radius_y=6))
     reg = imviz_helper.app.get_subsets("Subset 1")
     ellipse1 = EllipsePixelRegion(center=PixCoord(x=3, y=3),
                                   width=6, height=12, angle=0.0 * u.deg)
     assert reg[-1] == {'name': 'EllipticalROI', 'glue_state': 'AndNotState', 'region': ellipse1,
                        'subset_state': reg[-1]['subset_state']}
 
+    imviz_helper.app.session.edit_subset_mode.mode = OrMode
+    viewer.apply_roi(CircularAnnulusROI(xc=5, yc=5, inner_radius=2.5, outer_radius=5))
+    reg = imviz_helper.app.get_subsets("Subset 1")
+    ann1 = CircleAnnulusPixelRegion(center=PixCoord(x=5, y=5), inner_radius=2.5, outer_radius=5)
+    assert reg[-1] == {'name': 'CircularAnnulusROI', 'glue_state': 'OrState', 'region': ann1,
+                       'subset_state': reg[-1]['subset_state']}
+
     subset_plugin = imviz_helper.app.get_tray_item_from_name('g-subset-plugin')
     assert subset_plugin.subset_selected == "Subset 1"
-    assert subset_plugin.subset_types == ['CircularROI', 'RectangularROI', 'EllipticalROI']
-    assert subset_plugin.glue_state_types == ['AndState', 'AndNotState', 'AndNotState']
+    assert subset_plugin.subset_types == ['CircularROI', 'RectangularROI', 'EllipticalROI',
+                                          'CircularAnnulusROI']
+    assert subset_plugin.glue_state_types == ['AndState', 'AndNotState', 'AndNotState', 'OrState']
 
 
 def test_with_invalid_subset_name(cubeviz_helper):

diff --git a/pyproject.toml b/pyproject.toml
@@ -13,7 +13,7 @@ dependencies = [
     "bqplot>=0.12.37",
     "bqplot-image-gl>=1.4.11",
     "glue-core>=1.11",
-    "glue-jupyter>=0.16.3",
+    "glue-jupyter>=0.17",
     "echo>=0.5.0",
     "ipykernel>=6.19.4",
     "ipyvue>=1.6",
@@ -26,7 +26,7 @@ dependencies = [
     "specutils>=1.9",
     "specreduce>=1.3.0,<1.4.0",
     "photutils>=1.4",
-    "glue-astronomy>=0.9",
+    "glue-astronomy>=0.10",
     "asteval>=0.9.23",
     "idna",
     "vispy>=0.6.5",