Aperture live-preview plugin marks (spacetelescope#2664)

* basic infrastructure for ApertureSelect component with associated marks * multiple subset support * radius_factor support for upcoming cones * multiviewer support * update mark when changing link type * changelog entry * code cleanup * simplify coords logic thanks to suggestion by @pllim * support rectangles and ellipses * and rename radius_factor > scale_factor * integrate into cubeviz spectral extraction for future cone support * basic test coverage for cone visualization * update docstrings * update marks on change to subset * revert simplification so that marks update correctly on change to ref * Apply suggestions from code review Co-authored-by: P. L. Lim <[email protected]> * update marks when selected dataset is changed (including hiding when no dataset is selected) * fix aperture preview in multiple viewers with different rotations --------- Co-authored-by: P. L. Lim <[email protected]>
gibsongreen · Feb 12, 2024 · d30c0fd · d30c0fd
1 parent 08cc268
commit d30c0fd
Show file tree

Hide file tree

Showing 12 changed files with 459 additions and 51 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -11,6 +11,8 @@ New Features
 
 - Opacity for spatial subsets is now adjustable from within Plot Options. [#2663]
 
+- Live-preview of aperture selection in plugins. [#2664]
+
 Cubeviz
 ^^^^^^^
 - Calculated moments can now be output in velocity units. [#2584, #2588, #2665]
@@ -48,6 +50,9 @@ API Changes
 Cubeviz
 ^^^^^^^
 
+- ``spatial_subset`` in the spectral extraction plugin is now renamed to ``aperture`` and the deprecated name will
+  be removed in a future release. [#2664]
+
 Imviz
 ^^^^^
 

diff --git a/docs/dev/ui_style_guide.rst b/docs/dev/ui_style_guide.rst
@@ -22,7 +22,7 @@ try to adhere to the following principles:
   components are necessary in a single row.  Always emphasize readability at the default/minimum
   width of the plugin tray, rather than using columns that result in a ton of text overflow.
 * Use ``<v-row justify="end">`` to align content to the right (such as action buttons).
-* Action buttons should use ``<plugin-action-button :results_isolated_to_plugin="true/false">text</v-btn>``
+* Action buttons should use ``<plugin-action-button :results_isolated_to_plugin="true/false">text</plugin-action-button>``
   to control the color depending on whether the button affects things outside the plugin itself
   (adding/modifying data collection or subset entries, etc) or are isolated to within the plugin
   (adding model components in model fitting, etc).  These buttons can be wrapped in tooltip components

diff --git a/jdaviz/configs/cubeviz/plugins/slice/slice.py b/jdaviz/configs/cubeviz/plugins/slice/slice.py
@@ -11,7 +11,8 @@
 from specutils.spectra.spectrum1d import Spectrum1D
 
 from jdaviz.core.events import (AddDataMessage, SliceToolStateMessage,
-                                SliceSelectSliceMessage, GlobalDisplayUnitChanged)
+                                SliceSelectSliceMessage, SliceWavelengthUpdatedMessage,
+                                GlobalDisplayUnitChanged)
 from jdaviz.core.registries import tray_registry
 from jdaviz.core.template_mixin import PluginTemplateMixin
 from jdaviz.core.user_api import PluginUserApi
@@ -217,6 +218,10 @@ def _on_slider_updated(self, event):
         for viewer in self._indicator_viewers:
             viewer._update_slice_indicator(value)
 
+        self.hub.broadcast(SliceWavelengthUpdatedMessage(slice=value,
+                                                         wavelength=self.wavelength,
+                                                         sender=self))
+
     def vue_goto_first(self, *args):
         if self.is_playing:
             return

diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py
@@ -5,17 +5,19 @@
 import numpy as np
 import astropy
 import astropy.units as u
+from astropy.utils.decorators import deprecated
 from astropy.nddata import (
     NDDataArray, StdDevUncertainty, NDUncertainty
 )
-from traitlets import Bool, List, Unicode, observe
+from traitlets import Bool, Float, List, Unicode, observe
 
-from jdaviz.core.events import SnackbarMessage
+from jdaviz.core.custom_traitlets import FloatHandleEmpty
+from jdaviz.core.events import SnackbarMessage, SliceWavelengthUpdatedMessage
 from jdaviz.core.registries import tray_registry
 from jdaviz.core.template_mixin import (PluginTemplateMixin,
                                         DatasetSelectMixin,
                                         SelectPluginComponent,
-                                        SpatialSubsetSelectMixin,
+                                        ApertureSubsetSelectMixin,
                                         AddResultsMixin,
                                         with_spinner)
 from jdaviz.core.user_api import PluginUserApi
@@ -30,8 +32,8 @@
 @tray_registry(
     'cubeviz-spectral-extraction', label="Spectral Extraction", viewer_requirements='spectrum'
 )
-class SpectralExtraction(PluginTemplateMixin, DatasetSelectMixin,
-                         SpatialSubsetSelectMixin, AddResultsMixin):
+class SpectralExtraction(PluginTemplateMixin, ApertureSubsetSelectMixin,
+                         DatasetSelectMixin, AddResultsMixin):
     """
     See the :ref:`Spectral Extraction Plugin Documentation <spex>` for more details.
 
@@ -41,12 +43,20 @@ class SpectralExtraction(PluginTemplateMixin, DatasetSelectMixin,
     * :meth:`~jdaviz.core.template_mixin.PluginTemplateMixin.show`
     * :meth:`~jdaviz.core.template_mixin.PluginTemplateMixin.open_in_tray`
     * :meth:`~jdaviz.core.template_mixin.PluginTemplateMixin.close_in_tray`
-    * ``spatial_subset`` (:class:`~jdaviz.core.template_mixin.SubsetSelect`):
-      Subset to use for the spectral extraction, or ``No Subset``.
+    * ``aperture`` (:class:`~jdaviz.core.template_mixin.SubsetSelect`):
+      Subset to use for the spectral extraction, or ``Entire Cube``.
     * ``add_results`` (:class:`~jdaviz.core.template_mixin.AddResults`)
     * :meth:`collapse`
     """
     template_file = __file__, "spectral_extraction.vue"
+    uses_active_status = Bool(True).tag(sync=True)
+
+    # feature flag for cone support
+    dev_cone_support = Bool(False).tag(sync=True)
+    wavelength_dependent = Bool(False).tag(sync=True)
+    reference_wavelength = FloatHandleEmpty().tag(sync=True)
+    slice_wavelength = Float().tag(sync=True)
+
     function_items = List().tag(sync=True)
     function_selected = Unicode('Sum').tag(sync=True)
     filename = Unicode().tag(sync=True)
@@ -68,6 +78,12 @@ def __init__(self, *args, **kwargs):
 
         self.extracted_spec = None
 
+        # TODO: in the future this could be generalized with support in SelectPluginComponent
+        self.aperture._default_text = 'Entire Cube'
+        self.aperture._manual_options = ['Entire Cube']
+        self.aperture.items = [{"label": "Entire Cube"}]
+        self.aperture.select_default()
+
         self.function = SelectPluginComponent(
             self,
             items='function_items',
@@ -77,6 +93,9 @@ def __init__(self, *args, **kwargs):
         self._set_default_results_label()
         self.add_results.viewer.filters = ['is_spectrum_viewer']
 
+        self.session.hub.subscribe(self, SliceWavelengthUpdatedMessage,
+                                   handler=self._on_slice_changed)
+
         if ASTROPY_LT_5_3_2:
             self.disabled_msg = "Spectral Extraction in Cubeviz requires astropy>=5.3.2"
 
@@ -95,11 +114,44 @@ def user_api(self):
         return PluginUserApi(
             self,
             expose=(
-                'function', 'spatial_subset',
+                'function', 'spatial_subset', 'aperture',
                 'add_results', 'collapse_to_spectrum'
             )
         )
 
+    @property
+    @deprecated(since="3.9", alternative="aperture")
+    def spatial_subset(self):
+        return self.user_api.aperture
+
+    @property
+    def slice_plugin(self):
+        return self.app._jdaviz_helper.plugins['Slice']
+
+    @observe('wavelength_dependent')
+    def _wavelength_dependent_changed(self, *args):
+        if self.wavelength_dependent:
+            self.reference_wavelength = self.slice_plugin.wavelength
+        # NOTE: this can be redundant in the case where reference_wavelength changed and triggers
+        # the observe, but we need to ensure it is updated if reference_wavelength is unchanged
+        self._update_mark_scale()
+
+    def _on_slice_changed(self, msg):
+        self.slice_wavelength = msg.wavelength
+
+    def vue_goto_reference_wavelength(self, *args):
+        self.slice_plugin.wavelength = self.reference_wavelength
+
+    def vue_adopt_slice_as_reference(self, *args):
+        self.reference_wavelength = self.slice_plugin.wavelength
+
+    @observe('reference_wavelength', 'slice_wavelength')
+    def _update_mark_scale(self, *args):
+        if not self.wavelength_dependent:
+            self.aperture.scale_factor = 1.0
+            return
+        self.aperture.scale_factor = self.slice_wavelength/self.reference_wavelength
+
     @with_spinner()
     def collapse_to_spectrum(self, add_data=True, **kwargs):
         """
@@ -128,12 +180,12 @@ def collapse_to_spectrum(self, add_data=True, **kwargs):
         # defaults to ``No Subset``). Since the Cubeviz parser puts the fluxes
         # and uncertainties in different glue Data objects, we translate the spectral
         # cube and its uncertainties into separate NDDataArrays, then combine them:
-        if self.spatial_subset_selected != self.spatial_subset.default_text:
+        if self.aperture.selected != self.aperture.default_text:
             nddata = spectral_cube.get_subset_object(
-                subset_id=self.spatial_subset_selected, cls=NDDataArray
+                subset_id=self.aperture.selected, cls=NDDataArray
             )
             uncertainties = uncert_cube.get_subset_object(
-                subset_id=self.spatial_subset_selected, cls=StdDevUncertainty
+                subset_id=self.aperture.selected, cls=StdDevUncertainty
             )
         else:
             nddata = spectral_cube.get_object(cls=NDDataArray)
@@ -249,15 +301,15 @@ def _save_extracted_spec_to_fits(self, overwrite=False, *args):
             f"Extracted spectrum saved to {os.path.abspath(filename)}",
                            sender=self, color="success"))
 
-    @observe('spatial_subset_selected')
+    @observe('aperture_selected')
     def _set_default_results_label(self, event={}):
         label = "Spectral extraction"
 
         if (
-            hasattr(self, 'spatial_subset') and
-            self.spatial_subset.selected != self.spatial_subset.default_text
+            hasattr(self, 'aperture') and
+            self.aperture.selected != self.aperture.default_text
         ):
-            label += f' ({self.spatial_subset_selected})'
+            label += f' ({self.aperture_selected})'
         self.results_label_default = label
 
 

diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.vue
@@ -2,6 +2,9 @@
   <j-tray-plugin
     description='Extract a spectrum from a spectral cube.'
     :link="docs_link || 'https://jdaviz.readthedocs.io/en/'+vdocs+'/'+config+'/plugins.html#spectral-extraction'"
+    :uses_active_status="uses_active_status"
+    @plugin-ping="plugin_ping($event)"
+    :keep_active.sync="keep_active"
     :popout_button="popout_button"
     :disabled_msg="disabled_msg">
 
@@ -18,13 +21,53 @@
     </v-row>
 
     <plugin-subset-select 
-      :items="spatial_subset_items"
-      :selected.sync="spatial_subset_selected"
+      :items="aperture_items"
+      :selected.sync="aperture_selected"
       :show_if_single_entry="true"
-      label="Spatial region"
+      label="Spatial aperture"
       hint="Select a spatial region to extract its spectrum."
     />
 
+    <div v-if="aperture_selected !== 'Entire Cube' && dev_cone_support">
+      <v-alert type='warning'>cone support is under active development and hidden from users</v-alert>
+      <v-row>
+        <v-switch
+          v-model="wavelength_dependent"
+          label="Wavelength dependent"
+          hint="Vary aperture linearly with wavelength"
+          persistent-hint>
+        </v-switch>
+      </v-row>
+      <div v-if="wavelength_dependent">
+        <v-row justify="end">
+          <j-tooltip tooltipcontent="Adopt the current slice as the reference wavelength">
+            <plugin-action-button :results_isolated_to_plugin="true" @click="adopt_slice_as_reference">
+              Adopt Current Slice
+            </plugin-action-button>
+          </j-tooltip>
+        </v-row>
+        <v-row>
+          <v-text-field
+            v-model.number="reference_wavelength"
+            type="number"
+            :step="0.1"
+            class="mt-0 pt-0"
+            label="Wavelength"
+            hint="Wavelength at which the aperture matches the selected subset."
+            persistent-hint
+          ></v-text-field>
+        </v-row>
+        <v-row justify="end">
+          <j-tooltip tooltipcontent="Select the slice nearest the reference wavelength">
+            <plugin-action-button :results_isolated_to_plugin="true" @click="goto_reference_wavelength">
+              Slice to Reference Wavelength
+            </plugin-action-button>
+          </j-tooltip>
+        </v-row>
+      </div>
+
+    </div>
+
     <plugin-add-results
       :label.sync="results_label"
       :label_default="results_label_default"

diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py
@@ -85,7 +85,7 @@ def test_gauss_smooth_before_spec_extract(cubeviz_helper, spectrum1d_cube_with_u
     extract_plugin.function = "Sum"
     expected_uncert = 2
 
-    extract_plugin.spatial_subset = 'Subset 1'
+    extract_plugin.aperture = 'Subset 1'
     collapsed_spec = extract_plugin.collapse_to_spectrum()
 
     # this single pixel has two wavelengths, and all uncertainties are unity
@@ -94,7 +94,7 @@ def test_gauss_smooth_before_spec_extract(cubeviz_helper, spectrum1d_cube_with_u
     assert np.all(np.equal(collapsed_spec.uncertainty.array, 1))
 
     # this two-pixel region has four unmasked data points per wavelength:
-    extract_plugin.spatial_subset = 'Subset 2'
+    extract_plugin.aperture = 'Subset 2'
     collapsed_spec_2 = extract_plugin.collapse_to_spectrum()
     assert np.all(np.equal(collapsed_spec_2.uncertainty.array, expected_uncert))
 
@@ -129,7 +129,7 @@ def test_subset(
     plg.function = function
 
     # single pixel region:
-    plg.spatial_subset = 'Subset 1'
+    plg.aperture = 'Subset 1'
     collapsed_spec_1 = plg.collapse_to_spectrum()
 
     # this single pixel has two wavelengths, and all uncertainties are unity
@@ -138,7 +138,7 @@ def test_subset(
     assert np.all(np.equal(collapsed_spec_1.uncertainty.array, 1))
 
     # this two-pixel region has four unmasked data points per wavelength:
-    plg.spatial_subset = 'Subset 2'
+    plg.aperture = 'Subset 2'
     collapsed_spec_2 = plg.collapse_to_spectrum()
 
     assert np.all(np.equal(collapsed_spec_2.uncertainty.array, expected_uncert))
@@ -192,3 +192,42 @@ def test_save_collapsed_to_fits(cubeviz_helper, spectrum1d_cube_with_uncerts, tm
     with pytest.raises(ValueError, match="Invalid path=/this/path/doesnt"):
         extract_plugin._obj.vue_save_as_fits()
     extract_plugin._obj.filename == fname  # set back to original filename
+
+
+def test_aperture_markers(cubeviz_helper, spectrum1d_cube):
+
+    cubeviz_helper.load_data(spectrum1d_cube)
+    cubeviz_helper.load_regions([CirclePixelRegion(PixCoord(0.5, 0), radius=1.2)])
+
+    extract_plg = cubeviz_helper.plugins['Spectral Extraction']
+    slice_plg = cubeviz_helper.plugins['Slice']
+
+    mark = extract_plg.aperture.marks[0]
+    assert not mark.visible
+    with extract_plg.as_active():
+        assert mark.visible
+        assert not len(mark.x)
+
+        extract_plg.aperture = 'Subset 1'
+        before_x = mark.x
+        assert len(before_x) > 0
+
+        # sample cube only has 2 slices with wavelengths [4.62280007e-07 4.62360028e-07] m
+        slice_plg.slice = 1
+        assert mark.x[1] == before_x[1]
+
+        slice_plg.slice = 0
+        extract_plg._obj.dev_cone_support = True
+        extract_plg._obj.wavelength_dependent = True
+        assert mark.x[1] == before_x[1]
+
+        slice_plg.slice = 1
+        assert mark.x[1] != before_x[1]
+
+        extract_plg._obj.vue_goto_reference_wavelength()
+        assert slice_plg.slice == 0
+
+        slice_plg.slice = 1
+        extract_plg._obj.vue_adopt_slice_as_reference()
+        extract_plg._obj.vue_goto_reference_wavelength()
+        assert slice_plg.slice == 1
diff --git a/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_helper.py b/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_helper.py
@@ -27,6 +27,9 @@ def test_plugin_user_apis(cubeviz_helper):
     for plugin_name, plugin_api in cubeviz_helper.plugins.items():
         plugin = plugin_api._obj
         for attr in plugin_api._expose:
+            if plugin_name == 'Spectral Extraction' and attr == 'spatial_subset':
+                # deprecated, so would raise a deprecation warning
+                continue
             assert hasattr(plugin, attr)