add get_subsets_as_regions method to subset plugin, deprecate get_int…

…eractive_regions and get_spectral_regions (#3340)

* new get subset method

* rename to get_regions

* .

* test

* link follow up issue when trying to use import_region instead of apply_interactive_region

* remove _obj when using get_regions

* add get_regions to doc

* addl review

CHANGES.rst

@@ -43,6 +43,9 @@ API Changes
   ``metadata_plugin.meta`` instead, which will return a Python dictionary instead of
   list of tuples. [#3292]
 
+- Add ``get_regions`` method to subset plugin to retrieve spatial/spectral subsets as
+  ``regions`` or ``SpectralRegions``, deprecate ``get_interactive_regions`` and ``get_spectral_regions``. [#3340]
+
 Cubeviz
 ^^^^^^^
 

jdaviz/configs/cubeviz/plugins/tests/test_regions.py

@@ -42,7 +42,7 @@ def test_regions_pixel(self):
             [my_reg], return_bad_regions=True)
         assert len(bad_regions) == 0
         self.verify_region_loaded('Subset 1', count=1)
-        assert len(self.cubeviz.get_interactive_regions()) == 1
+        assert len(self.cubeviz.plugins['Subset Tools'].get_regions()) == 1
 
     def test_regions_sky_has_wcs(self):
         sky = SkyCoord(205.4397, 27.0035, unit='deg')
@@ -63,10 +63,14 @@ def test_spatial_spectral_mix(self):
                                                                           4.896 * unit))
         self.cubeviz.app.session.edit_subset_mode.edit_subset = None
 
-        # Get interactive spatial regions only.
-        spatial_subsets = self.cubeviz.get_interactive_regions()
-        assert list(spatial_subsets.keys()) == ['Subset 1'], spatial_subsets
-        assert isinstance(spatial_subsets['Subset 1'], EllipsePixelRegion)
+        # Get spatial regions only.
+        st = self.cubeviz.plugins['Subset Tools']._obj
+        spatial_subsets_as_regions = st.get_regions(region_type='spatial')
+        assert list(spatial_subsets_as_regions.keys()) == ['Subset 1'], spatial_subsets_as_regions
+        assert isinstance(spatial_subsets_as_regions['Subset 1'], EllipsePixelRegion)
+        # ensure agreement between app.get_subsets and subset_tools.get_regions
+        ss = self.cubeviz.app.get_subsets()
+        assert ss['Subset 1'][0]['region'] == spatial_subsets_as_regions['Subset 1']
 
         # NOTE: This does not test that spectrum from Subset is actually scientifically accurate.
         # Get spectral regions only.

jdaviz/configs/default/plugins/subset_tools/subset_tools.py

@@ -9,7 +9,7 @@
                                         ReplaceMode, XorMode, NewMode)
 from glue.core.roi import CircularROI, CircularAnnulusROI, EllipticalROI, RectangularROI
 from glue.core.subset import (RoiSubsetState, RangeSubsetState, CompositeSubsetState,
-                              MaskSubsetState)
+                              MaskSubsetState, Subset)
 from glue.icons import icon_path
 from glue_jupyter.widgets.subset_mode_vuetify import SelectionModeMenu
 from glue_jupyter.common.toolbar_vuetify import read_icon
@@ -83,6 +83,7 @@ class SubsetTools(PluginTemplateMixin):
     * :meth:`get_center`
     * :meth:`set_center`
     * :meth:`import_region`
+    * :meth:`get_regions`
     """
     template_file = __file__, "subset_tools.vue"
     select = List([]).tag(sync=True)
@@ -165,12 +166,110 @@ def __init__(self, *args, **kwargs):
 
     @property
     def user_api(self):
-        expose = ['subset', 'combination_mode',
-                  'recenter_dataset', 'recenter',
-                  'get_center', 'set_center',
-                  'import_region']
+        expose = ['subset', 'combination_mode', 'recenter_dataset', 'recenter',
+                  'get_center', 'set_center', 'import_region', 'get_regions']
         return PluginUserApi(self, expose)
 
+    def get_regions(self, region_type=None, list_of_subset_labels=None,
+                    use_display_units=False):
+        """
+        Return spatial and/or spectral subsets of ``region_type`` (spatial or
+        spectral, default both) as ``regions`` or ``SpectralRegions`` objects,
+        respectivley.
+
+        Parameters
+        ----------
+        region_type : str or None, optional
+            Specifies the type of subsets to retrieve. Options are ``spatial``
+            to retrieve only spatial subsets, ``spectral`` to retrieve only
+            spectral subsets or ``None`` (default) to retrieve both spatial
+            and spectral subsets, when relevent to the current configuration.
+
+        list_of_subset_labels : list of str or None, optional
+            If specified, only subsets matching these labels will be included.
+            If not specified, all subsets matching the ``region_type`` will be
+            returned.
+
+        use_display_units : bool, optional
+            (For spectral subsets) If False (default), subsets are returned in
+            the native data unit. If True, subsets are returned in the spectral
+            axis display unit set in the Unit Conversion plugin.
+
+        Returns
+        -------
+        regions : dict
+            A dictionary mapping subset labels to their respective ``regions``
+            objects (for spatial regions) or ``SpectralRegions`` objects
+            (for spectral regions).
+        """
+
+        if region_type is not None:
+            region_type = region_type.lower()
+            if region_type not in ['spectral', 'spatial']:
+                raise ValueError("`region_type` must be 'spectral', 'spatial', or None for any.")
+            if ((self.config == 'imviz' and region_type == 'spectral') or
+               (self.config == 'specviz' and region_type == 'spatial')):
+                raise ValueError(f"No {region_type} subests in {self.config}.")
+            region_type = [region_type]
+
+        else:  # determine which subset types should be returned by config, if type not specified
+            if self.config == 'imviz':
+                region_type = ['spatial']
+            elif self.config == 'specviz':
+                region_type = ['spectral']
+            else:
+                region_type = ['spatial', 'spectral']
+
+        regions = {}
+
+        if 'spatial' in region_type:
+            from glue_astronomy.translators.regions import roi_subset_state_to_region
+
+            failed_regs = set()
+            to_sky = self.app._align_by == 'wcs'
+
+            # Subset is global, so we just use default viewer.
+            for lyr in self.app._jdaviz_helper.default_viewer._obj.layers:
+                if (not hasattr(lyr, 'layer') or not isinstance(lyr.layer, Subset)
+                        or lyr.layer.ndim not in (2, 3)):
+                    continue
+
+                subset_data = lyr.layer
+                subset_label = subset_data.label
+
+                # TODO: Remove this when Jdaviz support round-tripping, see
+                # https://github.com/spacetelescope/jdaviz/pull/721
+                if not subset_label.startswith('Subset'):
+                    continue
+                try:
+                    if self.app.config == "imviz" and to_sky:
+                        region = roi_subset_state_to_region(subset_data.subset_state,
+                                                            to_sky=to_sky)
+                    else:
+                        region = subset_data.data.get_selection_definition(
+                            subset_id=subset_label, format='astropy-regions')
+                except (NotImplementedError, ValueError):
+                    failed_regs.add(subset_label)
+                else:
+                    regions[subset_label] = region
+
+            if len(failed_regs) > 0:
+                self.app.hub.broadcast(SnackbarMessage(
+                    f"Regions skipped: {', '.join(sorted(failed_regs))}",
+                    color="warning", timeout=8000, sender=self.app))
+
+        if 'spectral' in region_type:
+            spec_regions = self.app.get_subsets(spectral_only=True,
+                                                use_display_units=use_display_units)
+            if spec_regions:
+                regions.update(spec_regions)
+
+        # filter by list_of_subset_labels
+        if list_of_subset_labels is not None:
+            regions = {key: regions[key] for key in list_of_subset_labels}
+
+        return regions
+
     def _on_link_update(self, *args):
         """When linking is changed pixels<>wcs, change display units of the
         subset plugin from pixel (for pixel linking) to sky (for WCS linking).

jdaviz/configs/default/plugins/subset_tools/tests/test_subset_tools.py

@@ -4,6 +4,7 @@
 import numpy as np
 from astropy.nddata import NDData
 import astropy.units as u
+from regions import CirclePixelRegion, PixCoord
 from specutils import SpectralRegion
 from glue.core.roi import EllipticalROI, CircularROI, CircularAnnulusROI, RectangularROI
 from glue.core.edit_subset_mode import ReplaceMode, OrMode
@@ -226,3 +227,91 @@ def test_import_spectral_regions_file(cubeviz_helper, spectrum1d_cube, tmp_path)
 
     with pytest.raises(ValueError, match='\'test\' not one of'):
         plg.combination_mode = 'test'
+
+
+def test_get_regions(cubeviz_helper, spectrum1d_cube, imviz_helper):
+
+    """Test Subset Tools.get regions."""
+
+    cubeviz_helper.load_data(spectrum1d_cube)
+    plg = cubeviz_helper.plugins['Subset Tools']
+
+    # load one spectral region, which will become 'Subset 1'
+    plg.import_region(SpectralRegion(1 * u.um, 2 * u.um))
+
+    # load one spatial region, which will become 'Subset 2'
+    spatial_reg = CirclePixelRegion(center=PixCoord(x=2, y=2), radius=2)
+    plg.import_region(spatial_reg, combination_mode='new')
+
+    # call get_regions, which by default for cubeviz will return both
+    # spatial and spectral regions
+    all_regions = plg.get_regions()
+    assert len(all_regions) == 2
+
+    # make sure filtering by subset label works
+    only_s1 = plg.get_regions(list_of_subset_labels=['Subset 1'])
+    assert len(only_s1) == 1
+    assert only_s1['Subset 1']
+
+    # now specify region type and check output
+    spatial_regions = plg.get_regions(region_type='spatial')
+    assert len(spatial_regions) == 1
+    assert spatial_regions['Subset 2']
+    spectral_regions = plg.get_regions(region_type='spectral')
+    assert len(spectral_regions) == 1
+    assert spectral_regions['Subset 1']
+
+    # now test a composite spatial subset, make sure it is retrieved
+    sr1 = CirclePixelRegion(center=PixCoord(x=2.5, y=2.5), radius=2)
+    sr2 = CirclePixelRegion(center=PixCoord(x=2.5, y=3), radius=2)
+    plg.import_region(sr1, combination_mode='new')
+    plg.import_region(sr2, combination_mode='and')
+    spatial_regions = plg.get_regions(region_type='spatial')
+    assert spatial_regions['Subset 3']
+
+    # test errors
+    with pytest.raises(ValueError, match='No spectral subests in imviz.'):
+        imviz_helper.plugins['Subset Tools'].get_regions('spectral')
+    with pytest.raises(ValueError, match="`region_type` must be 'spectral', 'spatial', or None for any."):  # noqa E501
+        plg.get_regions(region_type='fail')
+
+
+@pytest.mark.xfail(reason='Unskip once issue XXXX is resolved.')
+def test_get_regions_composite(cubeviz_helper, spectrum1d_cube):
+    """
+    If you apply a circular subset mask to a circular subset to make a
+    composite subset, and they aren't exactly aligned at the center to form a
+    circular annulus, obtaining the region through ``get_interactive_regions``
+    (now deprecated, replaced with get_regions in Subset Tools) fails.
+    However, you can retrieve the compound subset as a ``region`` with
+    ``app.get_subsets``. This test ensures that a region is returned through
+    both ``app.get_subsets`` and ``get_regions``.
+    """
+    cubeviz_helper.load_data(spectrum1d_cube)
+    plg = cubeviz_helper.plugins['Subset Tools']
+
+    # For some reason, Subset 2 disappears after the third subset is applied
+    # when loaded this way. Uncomment to replace _apply_interactive_region once
+    # JDAT-5014 is resolved
+    # plg.import_region(CirclePixelRegion(center=PixCoord(x=96.0, y=96.0),
+    #                                     radius=45.0), combination_mode='new')
+    # plg.import_region(CirclePixelRegion(center=PixCoord(x=95.0, y=95.0),
+    #                                     radius=25.0), combination_mode='new')
+
+    # apply two circular subsets
+    cubeviz_helper._apply_interactive_region('bqplot:truecircle', (51, 51), (141, 141))
+    cubeviz_helper._apply_interactive_region('bqplot:truecircle', (70, 70), (120, 120))
+
+    # apply composite subset created from two existing circular subsets
+    subset_groups = cubeviz_helper.app.data_collection.subset_groups
+    new_subset = subset_groups[0].subset_state & ~subset_groups[1].subset_state
+    cubeviz_helper.default_viewer._obj.apply_subset_state(new_subset)
+
+    # make sure Subset 3, the composite subset, is retrieved.
+    regions = plg.get_regions()
+    ss_labels = ['Subset 1', 'Subset 2', 'Subset 3']
+    assert np.all([regions[ss] for ss in ss_labels])
+
+    # make sure the same regions are returned by app.get_subsets
+    get_subsets = cubeviz_helper.app.get_subsets()
+    assert np.all([get_subsets[ss][0]['region'] == regions[ss] for ss in ss_labels])

jdaviz/configs/imviz/tests/test_linking.py

@@ -120,7 +120,17 @@ def test_wcslink_affine_with_extras(self):
 
         # Ensure subsets are still there.
         all_labels = [layer.layer.label for layer in self.viewer.state.layers]
-        assert sorted(self.imviz.get_interactive_regions()) == ['Subset 1', 'Subset 2']
+        # Retrieved subsets as sky regions from Subset plugin, and ensure they
+        # match what was loaded and that they are in sky coordinates.
+        subset_as_regions = self.imviz.plugins['Subset Tools'].get_regions()
+        assert sorted(subset_as_regions) == ['Subset 1', 'Subset 2']
+        assert_allclose(subset_as_regions['Subset 1'].center.ra.deg, 337.519449)
+        assert_allclose(subset_as_regions['Subset 2'].center.ra.deg, 337.518498)
+        # ensure agreement between app.get_subsets and subset_tools.get_regions
+        ss = self.imviz.app.get_subsets(include_sky_region=True)
+        assert ss['Subset 1'][0]['sky_region'] == subset_as_regions['Subset 1']
+        assert ss['Subset 2'][0]['sky_region'] == subset_as_regions['Subset 2']
+
         assert 'MaskedSubset 1' in all_labels
         assert 'MaskedSubset 2' in all_labels
 

jdaviz/configs/imviz/tests/test_parser.py

@@ -9,7 +9,7 @@
 from astropy.wcs import WCS
 from gwcs import WCS as GWCS
 from numpy.testing import assert_allclose, assert_array_equal
-from regions import CirclePixelRegion, RectanglePixelRegion
+from regions import CirclePixelRegion, PixCoord, RectanglePixelRegion
 from skimage.io import imsave
 from stdatamodels import asdf_in_fits
 
@@ -259,10 +259,17 @@ def test_parse_jwst_nircam_level2(self, imviz_helper):
         imviz_helper._apply_interactive_region('bqplot:rectangle',
                                                (982, 1088),
                                                (1008, 1077))  # Background
-        subsets = imviz_helper.get_interactive_regions()
+        subsets = imviz_helper.plugins['Subset Tools'].get_regions()
         assert list(subsets.keys()) == ['Subset 1', 'Subset 2'], subsets
+        # check that retrieved subsets-as-regions from subset plugin match what was loaded.
         assert isinstance(subsets['Subset 1'], CirclePixelRegion)
+        assert subsets['Subset 1'].center == PixCoord(970.95, 1116.05)
         assert isinstance(subsets['Subset 2'], RectanglePixelRegion)
+        assert subsets['Subset 2'].center == PixCoord(995.0, 1082.5)
+        # ensure agreement between app.get_subsets and subset_tools.get_regions
+        ss = imviz_helper.app.get_subsets()
+        assert ss['Subset 1'][0]['region'] == subsets['Subset 1']
+        assert ss['Subset 2'][0]['region'] == subsets['Subset 2']
 
         # Test simple aperture photometry plugin.
         phot_plugin = imviz_helper.app.get_tray_item_from_name('imviz-aper-phot-simple')