remove spatial-subset from plot options and layer cycler

jdaviz/configs/default/plugins/model_fitting/model_fitting.py

@@ -1,4 +1,5 @@
 import re
+import logging
 import numpy as np
 from copy import deepcopy
 
@@ -12,7 +13,6 @@
 from jdaviz.core.template_mixin import (PluginTemplateMixin,
                                         SelectPluginComponent,
                                         SpectralSubsetSelectMixin,
-                                        SubsetSelect,
                                         DatasetSelectMixin,
                                         DatasetSpectralSubsetValidMixin,
                                         NonFiniteUncertaintyMismatchMixin,
@@ -54,8 +54,9 @@ class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
     * :meth:`~jdaviz.core.template_mixin.PluginTemplateMixin.close_in_tray`
     * ``dataset`` (:class:`~jdaviz.core.template_mixin.DatasetSelect`):
       Dataset to fit the model.
-    * ``spatial_subset`` (:class:`~jdaviz.core.template_mixin.SubsetSelect`):
-      Only exposed for Cubeviz.  Spatially collapsed spectrum to use to fit the model.
+    * ``spatial_subset``:
+      Deprecated as of 3.11.  Use the :ref:`Spectral Extraction Plugin <spectral-extraction>`
+      if necessary and select ``dataset`` and ``cube_fit`` accordingly.
     * ``spectral_subset`` (:class:`~jdaviz.core.template_mixin.SubsetSelect`)
     * ``model_component`` (:class:`~jdaviz.core.template_mixin.SelectPluginComponent`)
     * ``poly_order``
@@ -84,9 +85,6 @@ class ModelFitting(PluginTemplateMixin, DatasetSelectMixin,
     template_file = __file__, "model_fitting.vue"
     form_valid_model_component = Bool(False).tag(sync=True)
 
-    spatial_subset_items = List().tag(sync=True)
-    spatial_subset_selected = Unicode().tag(sync=True)
-
     # model components:
     model_comp_items = List().tag(sync=True)
     model_comp_selected = Unicode().tag(sync=True)
@@ -124,14 +122,6 @@ def __init__(self, *args, **kwargs):
         self.component_models = []
         self._initialized_models = {}
         self._display_order = False
-        if self.config == "cubeviz":
-            self.spatial_subset = SubsetSelect(self,
-                                               'spatial_subset_items',
-                                               'spatial_subset_selected',
-                                               default_text='Entire Cube',
-                                               filters=['is_spatial'])
-        else:
-            self.spatial_subset = None
 
         # create the label first so that when model_component defaults to the first selection,
         # the label automatically defaults as well
@@ -145,7 +135,10 @@ def __init__(self, *args, **kwargs):
 
         # when accessing the selected data, access the spectrum-viewer version
         self.dataset._viewers = [self._default_spectrum_viewer_reference_name]
-        # require entries to be in spectrum-viewer (not other cubeviz images, etc)
+        if self.config == 'cubeviz':
+            # allow cube fitting to data in the flux viewer (toggled via the cube_fit toggle)
+            self.dataset._viewers += [self._default_flux_viewer_reference_name]
+        # by default, require entries to be in spectrum-viewer (not other cubeviz images, etc)
         self.dataset.add_filter('layer_in_spectrum_viewer')
 
         self.equation = AutoTextField(self, 'model_equation', 'model_equation_default',
@@ -156,7 +149,7 @@ def __init__(self, *args, **kwargs):
 
         headers = ['model', 'data_label', 'spectral_subset', 'equation']
         if self.config == 'cubeviz':
-            headers += ['spatial_subset', 'cube_fit']
+            headers += ['cube_fit']
 
         self.table.headers_avail = headers
         self.table.headers_visible = headers
@@ -182,10 +175,27 @@ def _default_flux_viewer_reference_name(self):
             self.app._jdaviz_helper, '_default_flux_viewer_reference_name', 'flux-viewer'
         )
 
+    @observe('cube_fit')
+    def _cube_fit_changed(self, msg={}):
+        if self.cube_fit:
+            self.dataset.add_filter('layer_in_flux_viewer')
+            self.dataset.remove_filter('layer_in_spectrum_viewer')
+        else:
+            self.dataset.add_filter('layer_in_spectrum_viewer')
+            self.dataset.remove_filter('layer_in_flux_viewer')
+        self.dataset._clear_cache()
+
+    @property
+    def spatial_subset(self):
+        msg = "spatial_subset in no longer supported as of 3.11 and will be removed from the user API in a future release"  # noqa
+        logging.warning(f"DeprecationWarning: {msg}")
+        raise ValueError(msg)
+
     @property
     def user_api(self):
         expose = ['dataset']
         if self.config == "cubeviz":
+            # deprecated: spatial_subset deprecated in 3.11, so remove from user API as soon as 3.13
             expose += ['spatial_subset']
         expose += ['spectral_subset', 'model_component', 'poly_order', 'model_component_label',
                    'model_components', 'valid_model_components',
@@ -317,7 +327,7 @@ def _get_1d_spectrum(self):
         # retrieves the 1d spectrum
         return self.dataset.selected_spectrum(use_display_units=True)
 
-    @observe("dataset_selected", "spatial_subset_selected")
+    @observe("dataset_selected")
     def _dataset_selected_changed(self, event=None):
         """
         Callback method for when the user has selected data from the drop down
@@ -809,7 +819,6 @@ def calculate_fit(self, add_data=True):
                'spectral_subset': self.spectral_subset_selected,
                'equation': self.equation.value}
         if self.app.config == 'cubeviz':
-            row['spatial_subset'] = self.spatial_subset_selected
             row['cube_fit'] = self.cube_fit
 
         equation_components = self.equation_components
@@ -917,9 +926,8 @@ def _fit_model_to_cube(self, add_data):
         # Retrieve copy of the models with proper "fixed" dictionaries
         models_to_fit = self._reinitialize_with_fixed()
 
-        # Apply masks from selected subsets
-        for subset in [self.spatial_subset, self.spectral_subset]:
-            spec = self._apply_subset_masks(spec, subset)
+        # Apply masks from selected spectral subset
+        spec = self._apply_subset_masks(spec, self.spectral_subset)
 
         try:
             fitted_model, fitted_spectrum = fit_model_to_spectrum(
@@ -960,8 +968,7 @@ def _fit_model_to_cube(self, add_data):
     def _apply_subset_masks(self, spectrum, subset_component):
         """
         For a spectrum/spectral cube ``spectrum``, add a mask attribute
-        if none exists. Mask excludes non-selected spectral and/or
-        spatial subsets.
+        if none exists. Mask excludes non-selected spectral subsets.
         """
         # only look for a mask if there is a selected subset:
         if subset_component.selected == subset_component.default_text:

jdaviz/configs/default/plugins/model_fitting/model_fitting.vue

@@ -5,25 +5,26 @@
     :popout_button="popout_button"
     :scroll_to.sync="scroll_to">
 
-    <!-- for mosviz, the entries change on row change, so we want to always show the dropdown
-         to make sure that is clear -->
+    <v-row v-if="config=='cubeviz'">
+      <v-switch
+        v-model="cube_fit"
+        label="Cube Fit"
+        hint="Whether to fit to an extracted spectrum or full cube."
+        persistent-hint
+      ></v-switch>
+    </v-row>
+
+    <!-- for mosviz, the entries change on row change
+         for cubeviz, the entries change when toggling "cube fit"
+         so let's always show the dropdown for those cases to make the selection clear -->
     <plugin-dataset-select
       :items="dataset_items"
       :selected.sync="dataset_selected"
-      :show_if_single_entry="config=='mosviz'"
+      :show_if_single_entry="['mosviz', 'cubeviz'].indexOf(config) !== -1"
       label="Data"
       hint="Select the data set to be fitted."
     />
 
-    <plugin-subset-select
-      v-if="config=='cubeviz'"
-      :items="spatial_subset_items"
-      :selected.sync="spatial_subset_selected"
-      :show_if_single_entry="true"
-      label="Spatial region"
-      hint="Select spatial region to fit."
-    />
-
     <plugin-subset-select 
       :items="spectral_subset_items"
       :selected.sync="spectral_subset_selected"
@@ -235,15 +236,6 @@
       ></plugin-auto-label>
 
       <j-plugin-section-header>Fit Model</j-plugin-section-header>
-      <v-row>
-        <v-switch v-if="config=='cubeviz'"
-          v-model="cube_fit"
-          label="Cube Fit"
-          hint="Whether to fit to the collapsed spectrum or entire cube"
-          persistent-hint
-        ></v-switch>
-      </v-row>
-
       <v-row v-if="cube_fit">
         <span class="v-messages v-messages__message text--secondary">
             Note: cube fit results are not logged to table.

jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py

@@ -82,8 +82,8 @@ def test_parameter_retrieval(cubeviz_helper, spectral_cube_wcs):
     cubeviz_helper.load_data(Spectrum1D(flux=flux * u.nJy, wcs=spectral_cube_wcs),
                              data_label='test')
     plugin = cubeviz_helper.plugins["Model Fitting"]
-    plugin.create_model_component("Linear1D", "L")
     plugin.cube_fit = True
+    plugin.create_model_component("Linear1D", "L")
     with warnings.catch_warnings():
         warnings.filterwarnings('ignore', message='Model is linear in parameters.*')
         plugin.calculate_fit()

jdaviz/configs/default/plugins/model_fitting/tests/test_plugin.py

@@ -308,31 +308,6 @@ def test_subset_masks(cubeviz_helper, spectrum1d_cube_larger):
     # check that when no subset is selected, the spectral cube has no mask:
     p = cubeviz_helper.app.get_tray_item_from_name('g-model-fitting')
 
-    # Get the data object without collapsing the spectrum:
-    data = cubeviz_helper.app.data_collection[0].get_object(cls=Spectrum1D, statistic=None)
-    masked_data = p._apply_subset_masks(data, p.spatial_subset)
-
-    assert data.mask is None
-    assert masked_data.mask is None
-
-    # select the spatial subset, then show that mask is correct:
-    assert "Subset 1" in p.spatial_subset.choices
-    p.spatial_subset_selected = "Subset 1"
-
-    # Get the data object again (ensures mask == None)
-    data = cubeviz_helper.app.data_collection[0].get_object(
-        cls=Spectrum1D, statistic=None
-    )
-    subset = cubeviz_helper.app.data_collection[0].get_subset_object(
-        p.spatial_subset_selected, cls=Spectrum1D, statistic=None
-    )
-    masked_data = p._apply_subset_masks(data, p.spatial_subset)
-    expected_spatial_mask = np.ones(data.flux.shape).astype(bool)
-    expected_spatial_mask[:, :2, :] = False
-
-    assert np.all(masked_data.mask == expected_spatial_mask)
-    assert np.all(subset.mask == expected_spatial_mask)
-
     sv = cubeviz_helper.app.get_viewer('spectrum-viewer')
     # create a "Subset 2" entry in spectral dimension, selected "interactively"
     min_wavelength = 4625 * u.AA
@@ -344,10 +319,10 @@ def test_subset_masks(cubeviz_helper, spectrum1d_cube_larger):
 
     # Now create the new spectral subset:
     sv.apply_roi(XRangeROI(min=min_wavelength.to_value(u.m), max=max_wavelength.to_value(u.m)))
-    assert "Subset 2" in p.spectral_subset.choices
+    assert "Subset 1" in p.spectral_subset.choices
 
     # Select the spectral subset
-    p.spectral_subset_selected = "Subset 2"
+    p.spectral_subset_selected = "Subset 1"
 
     # Get the data object again (ensures mask == None)
     data = cubeviz_helper.app.data_collection[0].get_object(
@@ -364,19 +339,6 @@ def test_subset_masks(cubeviz_helper, spectrum1d_cube_larger):
     assert np.all(masked_data.mask == expected_spectral_mask)
     assert np.all(subset.mask == expected_spectral_mask)
 
-    # Get the data object again (ensures mask == None)
-    data = cubeviz_helper.app.data_collection[0].get_object(
-        cls=Spectrum1D, statistic=None
-    )
-
-    # apply both spectral+spatial masks:
-    masked_data = data
-    for subset in [p.spatial_subset, p.spectral_subset]:
-        masked_data = p._apply_subset_masks(masked_data, subset)
-
-    # Check that both masks are applied correctly
-    assert np.all(masked_data.mask == (expected_spectral_mask | expected_spatial_mask))
-
 
 def test_invalid_subset(specviz_helper, spectrum1d):
     # 6000-8000

jdaviz/configs/imviz/plugins/coords_info/coords_info.py

@@ -71,8 +71,6 @@ def __init__(self, *args, **kwargs):
         if self.app.config == 'imviz':
             # filter out scatter-plot entries (from add_markers API, for example)
             self.dataset.add_filter('is_image')
-        # we also want to include auto-collapsed spectra (spatial subsets)
-        self.dataset._cubeviz_include_spatial_subsets()
 
         # subscribe to mouse events on any new viewers
         self.hub.subscribe(self, ViewerAddedMessage, handler=self._on_viewer_added)

jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py

@@ -76,7 +76,7 @@ class LineAnalysis(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelect
     * :meth:`~jdaviz.core.template_mixin.PluginTemplateMixin.close_in_tray`
     * ``dataset`` (:class:`~jdaviz.core.template_mixin.DatasetSelect`):
       Dataset to use for computing line statistics.
-    * ``spatial_subset`` (:class:`~jdaviz.core.template_mixin.SubsetSelect`):
+    * ``spatial_subset``:
       Deprecated as of 3.11.  Use the :ref:`Spectral Extraction Plugin <spectral-extraction>`
       if necessary and select a spectrum as input to ``dataset``.
     * ``spectral_subset`` (:class:`~jdaviz.core.template_mixin.SubsetSelect`):

jdaviz/core/template_mixin.py

@@ -171,6 +171,17 @@ def spectrum_2d_viewer(self):
 
         return self.app.get_viewer(viewer_reference)
 
+    @cached_property
+    def flux_viewer(self):
+        if hasattr(self, '_default_flux_viewer_reference_name'):
+            viewer_reference = self._default_flux_viewer_reference_name
+        else:
+            viewer_reference = self.app._get_first_viewer_reference_name(
+                require_flux_viewer=True
+            )
+
+        return self.app.get_viewer(viewer_reference)
+
 
 class TemplateMixin(VuetifyTemplate, HubListener, ViewerPropertiesMixin):
     config = Unicode("").tag(sync=True)
@@ -849,6 +860,10 @@ def cached_properties(self):
     def add_filter(self, *filters):
         self.filters += [filter for filter in filters]
 
+    def remove_filter(self, *filters):
+        self.filters = [f for f in self.filters
+                        if (f not in filters and getattr(f, '__name__', '') not in filters)]
+
     @property
     def viewer_dicts(self):
         all_viewer_dicts = super().viewer_dicts
@@ -3291,27 +3306,6 @@ def __init__(self, plugin, items, selected,
         # initialize items from original viewers
         self._on_data_changed()
 
-    def _cubeviz_include_spatial_subsets(self):
-        """
-        Call this method to prepend spatial subsets to the list of datasets (and listen for newly
-        created spatial subsets).  For a single viewer, consider using LayerSelect instead.
-        """
-        if self.app.config != 'cubeviz':
-            return
-
-        # add additional callback for subsets
-        # We have to use SubsetUpdateMessage instead of SubsetCreateMessage to ensure it has already
-        # been added to data_collection.subset_groups.  To avoid extra calls to _on_data_changed
-        # for changes in style, etc, we'll try to filter out extra messages in advance.
-        def _subset_update(msg):
-            if msg.attribute == 'subset_state':
-                if get_subset_type(msg.subset) == 'spatial':
-                    self._on_data_changed()
-
-        self.hub.subscribe(self, SubsetUpdateMessage,
-                           handler=_subset_update)
-        self._include_spatial_subsets = True
-
     @property
     def default_data_cls(self):
         if self.app.config == 'imviz':
@@ -3397,6 +3391,12 @@ def layer_in_spectrum_2d_viewer(data):
                 return True
             return data.label in [l.layer.label for l in self.spectrum_2d_viewer.layers]  # noqa E741
 
+        def layer_in_flux_viewer(data):
+            if not len(self.app.get_viewer_reference_names()):
+                # then this is a bare Application object, so ignore this filter
+                return True
+            return data.label in [l.layer.label for l in self.flux_viewer.layers]  # noqa E741
+
         def is_trace(data):
             return hasattr(data, 'meta') and 'Trace' in data.meta
 
@@ -3436,10 +3436,6 @@ def _dc_to_dict(data):
         manual_items = [{'label': label} for label in self.manual_options]
         self.items = manual_items + [_dc_to_dict(data) for data in self.app.data_collection
                                      if self._is_valid_item(data)]
-        if getattr(self, '_include_spatial_subsets', False):
-            # allow for spatial subsets to be listed
-            self.items = self.items + [_dc_to_dict(subset) for subset in self.app.data_collection.subset_groups  # noqa
-                                       if get_subset_type(subset) == 'spatial']
         self._apply_default_selection()
         # future improvement: only clear cache if the selected data entry was changed?
         self._clear_cache(*self._cached_properties)