Refactor Unit Conversion plugin in Specviz (#2127)

* Added .ico and .svg files * app-wide display unit in specviz (#2048) * basic app-wide display unit in specviz (not all plugins are updated yet) * unit-aware select component which maps to glue-supported unit strings * implement use_display_units option for get_data, get_subsets Co-authored-by: Jesse Averbukh <[email protected]> Co-authored-by: Kyle Conroy <[email protected]> * bump glue-core to necessary version for display units * change PR number in changelog to main PR * Display units line analysis (#2128) * emit event when display unit changed * update line analysis for unit changes * cubeviz/slice plugin unit conversion support (we might want to generalize some of this logic more into the mark's _update_data when adding support for spectral lines as well) * Display units support for markers plugin (#2130) * update markers plugin for display units * move unit-updating logic to mark itself * LineUncertainties to be unit-aware * fix support for markers in cubeviz * display units: fix failing tests (#2132) * fix error raised when spectrum uncertainty is None * fix RTD build failure because of missing mixin available in import * fix test setting display unit now that um is preferred to micron * add disabled unit conversion plugin to mosviz * avoid unit-conversion error in cubeviz tests * bump versions of glue/glue-jupyter * fix deg intialization in cubeviz slice plugin * add LinesAutoUnit to __all__ so RTD can find it * add use_display_units kwarg to get_spectral_regions * fix setting display units for unitless data * Apply suggestions from code review Co-authored-by: P. L. Lim <[email protected]> * Display units: line lists (#2148) * BaseSpectrumVerticalLine to inherit unit-support from PluginMark * convert units on internal rest value * code cleanup * remove unit conversion plugin from all bug specviz * for initial implementation - eventually we'll want to implement across all plugins/viewers * get_display_unit fallback for configs without unit conversion plugin * Update glue-core dependency * Update Subset Tools plugin to use display units (#2195) * bump glue-core to necessary version for display units * cubeviz/slice plugin unit conversion support (we might want to generalize some of this logic more into the mark's _update_data when adding support for spectral lines as well) * display units: fix failing tests (#2132) * fix error raised when spectrum uncertainty is None * fix RTD build failure because of missing mixin available in import * fix test setting display unit now that um is preferred to micron * add disabled unit conversion plugin to mosviz * avoid unit-conversion error in cubeviz tests * bump versions of glue/glue-jupyter * fix deg intialization in cubeviz slice plugin * add LinesAutoUnit to __all__ so RTD can find it * add use_display_units kwarg to get_spectral_regions * fix setting display units for unitless data * Apply suggestions from code review Co-authored-by: P. L. Lim <[email protected]> * remove unit conversion plugin from all bug specviz * for initial implementation - eventually we'll want to implement across all plugins/viewers * Working on unit conversion handling in subset plugin * Get subset updating working in non-default units * Add changelog * Update jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue Co-authored-by: Kyle Conroy <[email protected]> --------- Co-authored-by: Kyle Conroy <[email protected]> Co-authored-by: P. L. Lim <[email protected]> * allow downstream apps to set what classes are considered native marks * Fix unit conversion for PluginMarks and y-limits (#2215) * Fix unit conversion for PluginMarks and y-limits * Only use equivalency if it's a spec viewer * Store wavelength unit on data update * Remove redundant line * Better check in marks * Replace subset_to_apply in docs where possible (#2223) * Replace subset_to_apply in docs where possible (#2223) * Avoid trying to convert units of empty marks arrays * Add open method to automatically detect config and load data * Update demo notebooks * Codestyle * Add autoconfig test coverage * Revert "Update demo notebooks" This reverts commit cc513b3. * Add mention of open to relevant example notebooks * Prepare to support open via cli * Remove the need to open file twice for autoconfig detection * Fix Helper vs App Docstring * TST: Change PIP_EXTRA_INDEX_URL because packages moved where they upload nightly builds. * Display units: model fitting (#2216) * model component compatibility based on display units * equation validity based on display units * allow re-estimating model parameters to update compat checks * test coverage * get_data(use_display_units) to pass spectral_density equivalency * handle whitespace in model equation * Add clarity for kwargs * plugin results: support overwriting when data loaded in multiple viewers (#2222) * reverts removal of call to set_plot_axes when loading data into viewer (#2235) * fixes a bug in the axes labels on cube viewers in cubeviz as well as setting the correct attributes in lcviz * Subsume failing identifier test into autoconfig test * Update subset plugin UI (#2234) * Update subset plugin UI * Apply suggestions from code review Co-authored-by: Kyle Conroy <[email protected]> --------- Co-authored-by: Kyle Conroy <[email protected]> * Changelog * Add docs page to explain how to create jdaviz-readable products (#2228) * Add page for data products * Update index * Fix title * Capital letters * Address first review comments * Edit format * Apply suggestions from code review Co-authored-by: P. L. Lim <[email protected]> * Including reference in import_data * Update docs/create_products.rst * Update docs/create_products.rst --------- Co-authored-by: Camilla Pacifici <[email protected]> Co-authored-by: P. L. Lim <[email protected]> Co-authored-by: Ricky O'Steen <[email protected]> * FEAT: Load annulus from file and allow IMPORT DATA to load reg files. Annulus support provided by glue-viz/glue#2403 and glue-viz/glue-astronomy#92 Add test for exception. Bump minimum requirements for glue-core and glue-astronomy. * disallow gaussian smooth output as input * BUG: Fix ellipse translation in app.get_subsets() * DOC: Use pydata-sphinx-theme (#2243) * DOC: Use pydata-sphinx-theme that has dark mode, modern design, and mobile friendly DOC: No longer need tomli DOC: Now requires sphinx-astropy 1.9.1 * Add cards to index page with icons * Remove iframe hardcoding so it scales in mobile mode * DOC: Insert small logo next to title instead of a giant logo above the title * Add app method to simplify spectral subset (#2237) * Add app method to simplify spectral subset Add simplify button to subset plugin * Fix xor bug exposed by simplify button * Fix style checks * Remove print statements * Add tooltip to simplify button * Make button appear only if the subset can be simplified * Make simplify button appear only when applicable * Apply suggestions from code review Co-authored-by: P. L. Lim <[email protected]> * Update changes file --------- Co-authored-by: P. L. Lim <[email protected]> * Fix missed conflict --------- Co-authored-by: Jennifer Kotler <[email protected]> Co-authored-by: P. L. Lim <[email protected]> Co-authored-by: Jesse Averbukh <[email protected]> Co-authored-by: Ricky O'Steen <[email protected]> Co-authored-by: Ricky O'Steen <[email protected]> Co-authored-by: Duy Nguyen <[email protected]> Co-authored-by: Camilla Pacifici <[email protected]> Co-authored-by: Camilla Pacifici <[email protected]>
spacetelescope · Jun 13, 2023 · 19f89e4 · 19f89e4
1 parent 2093786
commit 19f89e4
Show file tree

Hide file tree

Showing 40 changed files with 1,263 additions and 943 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -32,6 +32,8 @@ Specviz2d
 API Changes
 -----------
 
+- Subset Plugin now respects the chosen display unit after using Unit Conversion. [#2195]
+
 Cubeviz
 ^^^^^^^
 
@@ -44,6 +46,8 @@ Mosviz
 Specviz
 ^^^^^^^
 
+* Re-enabled unit conversion support. [#2127]
+
 Specviz2d
 ^^^^^^^^^
 

diff --git a/docs/specviz/plugins.rst b/docs/specviz/plugins.rst
@@ -118,15 +118,8 @@ To export the table into the notebook via the API, call
 Unit Conversion
 ===============
 
-.. note::
-
-    This plugin is temporarily disabled. Effort to improve it is being
-    tracked at https://github.com/spacetelescope/jdaviz/issues/1972 .
-
 The spectral flux density and spectral axis units can be converted
-using the Unit Conversion plugin.  The Spectrum1D object to be
-converted is the currently selected spectrum in the spectrum viewer :guilabel:`Data`
-icon in the viewer toolbar.
+using the Unit Conversion plugin.
 
 Select the frequency, wavelength, or energy unit in the
 :guilabel:`New Spectral Axis Unit` pulldown
@@ -135,11 +128,8 @@ Select the frequency, wavelength, or energy unit in the
 Select the flux density unit in the :guilabel:`New Flux Unit` pulldown
 (e.g., Jansky, W/(Hz/m2), ph/(Angstrom cm2 s)).
 
-The :guilabel:`Apply` button will convert the flux density and/or
-spectral axis units and create a new Spectrum1D object that
-is automatically switched to in the spectrum viewer.
-The name of the new Spectrum1D object is "_units_copy_" plus
-the flux and spectral units of the spectrum.
+Note that this affects the default units in all viewers and plugins, where applicable,
+but does not affect the underlying data.
 
 .. _line-lists:
 

diff --git a/jdaviz/app.py b/jdaviz/app.py
@@ -9,9 +9,9 @@
 from ipywidgets import widget_serialization
 import ipyvue
 
+from astropy import units as u
 from astropy.nddata import CCDData, NDData
 from astropy.io import fits
-from astropy import units as u
 from astropy.coordinates import Angle
 from astropy.time import Time
 from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion, EllipsePixelRegion
@@ -28,7 +28,7 @@
 from glue.config import colormaps, data_translator
 from glue.config import settings as glue_settings
 from glue.core import BaseData, HubListener, Data, DataCollection
-from glue.core.link_helpers import LinkSame
+from glue.core.link_helpers import LinkSame, LinkSameWithUnits
 from glue.plugins.wcs_autolinking.wcs_autolinking import WCSLink, IncompatibleWCS
 from glue.core.message import (DataCollectionAddMessage,
                                DataCollectionDeleteMessage,
@@ -39,6 +39,7 @@
 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_jupyter.app import JupyterApplication
 from glue_jupyter.common.toolbar_vuetify import read_icon
@@ -70,10 +71,57 @@
                  mask=['mask', 'dq'])
 
 
+@unit_converter('custom-jdaviz')
+class UnitConverterWithSpectral:
+
+    def equivalent_units(self, data, cid, units):
+        if cid.label == "flux":
+            eqv = u.spectral_density(1 * u.m)  # Value does not matter here.
+            list_of_units = set(list(map(str, u.Unit(units).find_equivalent_units(
+                include_prefix_units=True, equivalencies=eqv))) + [
+                    'Jy', 'mJy', 'uJy',
+                    'W / (m2 Hz)', 'W / (Hz m2)',  # Order is different in astropy v5.3
+                    'eV / (s m2 Hz)', 'eV / (Hz s m2)',
+                    'erg / (s cm2)',
+                    'erg / (s cm2 Angstrom)', 'erg / (Angstrom s cm2)',
+                    'erg / (s cm2 Hz)', 'erg / (Hz s cm2)',
+                    'ph / (s cm2 Angstrom)', 'ph / (Angstrom s cm2)',
+                    'ph / (s cm2 Hz)', 'ph / (Hz s cm2)'
+                ])
+        else:  # spectral axis
+            # prefer Hz over Bq and um over micron
+            exclude = {'Bq', 'micron'}
+            list_of_units = set(list(map(str, u.Unit(units).find_equivalent_units(
+                include_prefix_units=True, equivalencies=u.spectral())))) - exclude
+        return list_of_units
+
+    def to_unit(self, data, cid, values, original_units, target_units):
+        # Given a glue data object (data), a component ID (cid), the values
+        # to convert, and the original and target units of the values, this method
+        # should return the converted values. Note that original_units
+        # gives the units of the values array, which might not be the same
+        # as the original native units of the component in the data.
+        if cid.label == "flux":
+            spec = data.get_object(cls=Spectrum1D)
+            if len(values) == 2:
+                # Need this for setting the y-limits
+                spec_limits = [spec.spectral_axis[0].value, spec.spectral_axis[-1].value]
+                eqv = u.spectral_density(spec_limits*spec.spectral_axis.unit)
+            else:
+                eqv = u.spectral_density(spec.spectral_axis)
+        else:  # spectral axis
+            eqv = u.spectral()
+
+        return (values * u.Unit(original_units)).to_value(u.Unit(target_units), equivalencies=eqv)
+
+
 # Set default opacity for data layers to 1 instead of 0.8 in
 # some glue-core versions
 glue_settings.DATA_ALPHA = 1
 
+# Enable spectrum unit conversion.
+glue_settings.UNIT_CONVERTER = 'custom-jdaviz'
+
 custom_components = {'j-tooltip': 'components/tooltip.vue',
                      'j-external-link': 'components/external_link.vue',
                      'j-docs-link': 'components/docs_link.vue',
@@ -466,7 +514,7 @@ def _link_new_data(self, reference_data=None, data_to_be_linked=None):
         if isinstance(linked_data.coords, SpectralCoordinates):
             wc_old = ref_data.world_component_ids[-1]
             wc_new = linked_data.world_component_ids[0]
-            self.data_collection.add_link(LinkSame(wc_old, wc_new))
+            self.data_collection.add_link(LinkSameWithUnits(wc_old, wc_new))
             return
 
         try:
@@ -512,8 +560,8 @@ def _link_new_data(self, reference_data=None, data_to_be_linked=None):
                 else:
                     continue
 
-                links.append(LinkSame(ref_data.pixel_component_ids[ref_index],
-                                      linked_data.pixel_component_ids[linked_index]))
+                links.append(LinkSameWithUnits(ref_data.pixel_component_ids[ref_index],
+                                               linked_data.pixel_component_ids[linked_index]))
 
             dc.add_link(links)
 
@@ -852,7 +900,8 @@ def get_subsets_from_viewer(self, viewer_reference, data_label=None, subset_type
         return regions
 
     def get_subsets(self, subset_name=None, spectral_only=False,
-                    spatial_only=False, object_only=False, simplify_spectral=True):
+                    spatial_only=False, object_only=False, simplify_spectral=True,
+                    use_display_units=False):
         """
         Returns all branches of glue subset tree in the form that subset plugin can recognize.
 
@@ -869,6 +918,9 @@ def get_subsets(self, subset_name=None, spectral_only=False,
             leave out the region class name and glue_state.
         simplify_spectral : bool
             Return a composite spectral subset collapsed to a simplified SpectralRegion.
+        use_display_units: bool, optional
+            Whether to convert to the display units defined in the
+            :ref:`Unit Conversion <unit-conversion>` plugin.
 
         Returns
         -------
@@ -887,15 +939,17 @@ def get_subsets(self, subset_name=None, spectral_only=False,
             if isinstance(subset.subset_state, CompositeSubsetState):
                 # Region composed of multiple ROI or Range subset
                 # objects that must be traversed
-                subset_region = self.get_sub_regions(subset.subset_state, simplify_spectral)
+                subset_region = self.get_sub_regions(subset.subset_state,
+                                                     simplify_spectral, use_display_units)
             elif isinstance(subset.subset_state, RoiSubsetState):
                 # 3D regions represented as a dict including an
                 # AstropyRegion object if possible
                 subset_region = self._get_roi_subset_definition(subset.subset_state)
             elif isinstance(subset.subset_state, RangeSubsetState):
                 # 2D regions represented as SpectralRegion objects
                 subset_region = self._get_range_subset_bounds(subset.subset_state,
-                                                              simplify_spectral)
+                                                              simplify_spectral,
+                                                              use_display_units)
             else:
                 # subset.subset_state can be an instance of MaskSubsetState
                 # or something else we do not know how to handle
@@ -966,19 +1020,25 @@ def _remove_duplicate_bounds(self, spec_regions):
                 regions_no_dups += region
         return regions_no_dups
 
-    def _get_range_subset_bounds(self, subset_state, simplify_spectral=True):
+    def _get_range_subset_bounds(self, subset_state,
+                                 simplify_spectral=True, use_display_units=False):
         # TODO: Use global display units
         # units = dc[0].data.coords.spectral_axis.unit
         viewer = self.get_viewer(self._jdaviz_helper. _default_spectrum_viewer_reference_name)
         data = viewer.data()
-        if viewer:
-            units = u.Unit(viewer.state.x_display_unit)
-        elif data and len(data) > 0 and isinstance(data[0], Spectrum1D):
+        if data and len(data) > 0 and isinstance(data[0], Spectrum1D):
             units = data[0].spectral_axis.unit
         else:
             raise ValueError("Unable to find spectral axis units")
+        if use_display_units:
+            # converting may result in flipping order (wavelength <-> frequency)
+            ret_units = self._get_display_unit('spectral')
+            subset_bounds = [(subset_state.lo * units).to(ret_units, u.spectral()),
+                             (subset_state.hi * units).to(ret_units, u.spectral())]
+            spec_region = SpectralRegion(min(subset_bounds), max(subset_bounds))
+        else:
+            spec_region = SpectralRegion(subset_state.lo * units, subset_state.hi * units)
 
-        spec_region = SpectralRegion(subset_state.lo * units, subset_state.hi * units)
         if not simplify_spectral:
             return [{"name": subset_state.__class__.__name__,
                      "glue_state": subset_state.__class__.__name__,
@@ -1013,12 +1073,14 @@ def _get_roi_subset_definition(self, subset_state):
                  "region": roi_as_region,
                  "subset_state": subset_state}]
 
-    def get_sub_regions(self, subset_state, simplify_spectral=True):
+    def get_sub_regions(self, subset_state, simplify_spectral=True, use_display_units=False):
 
         if isinstance(subset_state, CompositeSubsetState):
             if subset_state and hasattr(subset_state, "state2") and subset_state.state2:
-                one = self.get_sub_regions(subset_state.state1, simplify_spectral)
-                two = self.get_sub_regions(subset_state.state2, simplify_spectral)
+                one = self.get_sub_regions(subset_state.state1,
+                                           simplify_spectral, use_display_units)
+                two = self.get_sub_regions(subset_state.state2,
+                                           simplify_spectral, use_display_units)
 
                 if isinstance(one, list) and "glue_state" in one[0]:
                     one[0]["glue_state"] = subset_state.__class__.__name__
@@ -1137,15 +1199,34 @@ def get_sub_regions(self, subset_state, simplify_spectral=True):
             else:
                 # This gets triggered in the InvertState case where state1
                 # is an object and state2 is None
-                return self.get_sub_regions(subset_state.state1, simplify_spectral)
+                return self.get_sub_regions(subset_state.state1,
+                                            simplify_spectral, use_display_units)
         elif subset_state is not None:
             # This is the leaf node of the glue subset state tree where
             # a subset_state is either ROI or Range.
             if isinstance(subset_state, RoiSubsetState):
                 return self._get_roi_subset_definition(subset_state)
 
             elif isinstance(subset_state, RangeSubsetState):
-                return self._get_range_subset_bounds(subset_state, simplify_spectral)
+                return self._get_range_subset_bounds(subset_state,
+                                                     simplify_spectral, use_display_units)
+
+    def _get_display_unit(self, axis):
+        if self._jdaviz_helper is None or self._jdaviz_helper.plugins.get('Unit Conversion') is None:  # noqa
+            # fallback on native units (unit conversion is not enabled)
+            if axis == 'spectral':
+                sv = self.get_viewer(self._jdaviz_helper._default_spectrum_viewer_reference_name)
+                return sv.data()[0].spectral_axis.unit
+            elif axis == 'flux':
+                sv = self.get_viewer(self._jdaviz_helper._default_spectrum_viewer_reference_name)
+                return sv.data()[0].flux.unit
+            else:
+                raise ValueError(f"could not find units for axis='{axis}'")
+        try:
+            return getattr(self._jdaviz_helper.plugins.get('Unit Conversion')._obj,
+                           f'{axis}_unit_selected')
+        except AttributeError:
+            raise ValueError(f"could not find display unit for axis='{axis}'")
 
     def simplify_spectral_subset(self, subset_name, att, overwrite=False):
         """
@@ -1363,19 +1444,6 @@ def add_data_to_viewer(self, viewer_reference, data_label,
 
         self.set_data_visibility(viewer_item, data_label, visible=visible, replace=clear_other_data)
 
-    def _set_plot_axes_labels(self, viewer_id):
-        """
-        Sets the plot axes labels to be the units of the data to be loaded.
-
-        Parameters
-        ----------
-        viewer_id : str
-            The UUID associated with the desired viewer item.
-        """
-        viewer = self._viewer_by_id(viewer_id)
-
-        viewer.set_plot_axes()
-
     def remove_data_from_viewer(self, viewer_reference, data_label):
         """
         Removes a data set from the specified viewer.
@@ -1716,6 +1784,7 @@ def set_data_visibility(self, viewer_reference, data_label, visible=True, replac
             [data] = [x for x in self.data_collection if x.label == data_label]
 
             viewer.add_data(data, percentile=95, color=viewer.color_cycler())
+            viewer.set_plot_axes()
 
             add_data_message = AddDataMessage(data, viewer,
                                               viewer_id=viewer_id,
@@ -1751,11 +1820,8 @@ def set_data_visibility(self, viewer_reference, data_label, visible=True, replac
         # active data.
         viewer_data_labels = [layer.layer.label for layer in viewer.layers]
         if len(viewer_data_labels) > 0 and getattr(self._jdaviz_helper, '_in_batch_load', 0) == 0:
-            active_data = self.data_collection[viewer_data_labels[0]]
-            if (hasattr(active_data, "_preferred_translation")
-                    and active_data._preferred_translation is not None):
-                self._set_plot_axes_labels(viewer_id)
-
+            # This "if" is nested on purpose to make parent "if" available
+            # for other configs in the future, as needed.
             if self.config == 'imviz':
                 viewer.on_limits_change()  # Trigger compass redraw
 

diff --git a/jdaviz/components/tooltip.vue b/jdaviz/components/tooltip.vue
@@ -76,7 +76,7 @@ const tooltips = {
   'plugin-plot-options-mixed-state': 'Current values are mixed, click to sync at shown value',
   'plugin-model-fitting-add-model': 'Create model component',
   'plugin-model-fitting-param-fixed': 'Check the box to freeze parameter value',
-  'plugin-model-fitting-reestimate-all': 'Re-estimate initial values based on the current data/subset selection for all free parameters',
+  'plugin-model-fitting-reestimate-all': 'Re-estimate initial values based on the current data/subset selection for all free parameters based on current display units',
   'plugin-model-fitting-reestimate': 'Re-estimate initial values based on the current data/subset selection for all free parameters in this component',
   'plugin-unit-conversion-apply': 'Apply unit conversion',
   'plugin-line-lists-load': 'Load list into "Loaded Lines" section of plugin',

diff --git a/jdaviz/configs/cubeviz/cubeviz.yaml b/jdaviz/configs/cubeviz/cubeviz.yaml
@@ -25,7 +25,6 @@ tray:
   - g-gaussian-smooth
   - g-collapse
   - g-model-fitting
-  - g-unit-conversion
   - g-line-list
   - specviz-line-analysis
   - cubeviz-moment-maps

diff --git a/jdaviz/configs/cubeviz/helper.py b/jdaviz/configs/cubeviz/helper.py
@@ -121,7 +121,7 @@ def specviz(self):
         return self._specviz
 
     def get_data(self, data_label=None, spatial_subset=None, spectral_subset=None, function=None,
-                 cls=None):
+                 cls=None, use_display_units=False):
         """
         Returns data with name equal to ``data_label`` of type ``cls`` with subsets applied from
         ``spatial_subset`` and/or ``spectral_subset`` using ``function`` if applicable.
@@ -162,7 +162,8 @@ def get_data(self, data_label=None, spatial_subset=None, spectral_subset=None, f
         elif function is False:
             function = None
         return self._get_data(data_label=data_label, spatial_subset=spatial_subset,
-                              spectral_subset=spectral_subset, function=function, cls=cls)
+                              spectral_subset=spectral_subset, function=function,
+                              cls=cls, use_display_units=use_display_units)
 
 
 def layer_is_cube_image_data(layer):