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]>

jdaviz/app.py

@@ -895,7 +895,8 @@ def get_subsets(self, subset_name=None, spectral_only=False,
         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 <unit-conversion> plugin.
+            Whether to convert to the display units defined in the
+            :ref:`Unit Conversion <unit-conversion>` plugin.
 
         Returns
         -------

jdaviz/configs/cubeviz/plugins/slice/slice.py

@@ -171,7 +171,8 @@ def _on_global_display_unit_changed(self, msg):
             return
         prev_unit = self.wavelength_unit
         self.wavelength_unit = msg.unit.to_string()
-        if self._x_all is None or prev_unit == '':
+        # original unit during init can be blank or deg (before axis is set correctly)
+        if self._x_all is None or prev_unit in ('deg', ''):
             return
         self._update_data((self._x_all * u.Unit(prev_unit)).to(msg.unit, u.spectral()))
 

jdaviz/configs/mosviz/mosviz.yaml

@@ -22,6 +22,7 @@ tray:
   - g-gaussian-smooth
   - g-slit-overlay
   - g-model-fitting
+  - g-unit-conversion
   - g-line-list
   - specviz-line-analysis
   - g-export-plot

jdaviz/configs/specviz/helper.py

@@ -128,18 +128,24 @@ def get_spectra(self, data_label=None, subset_to_apply=None, apply_slider_redshi
 
             return output_spectra
 
-    def get_spectral_regions(self):
+    def get_spectral_regions(self, use_display_units=False):
         """
         A simple wrapper around the app-level call to retrieve only spectral
         subsets, which are now returned as SpectralRegions by default.
 
+        Parameters
+        ----------
+        use_display_units : bool, optional
+            Whether to convert to the display units defined in the
+            :ref:`Unit Conversion <unit-conversion>` plugin.
+
         Returns
         -------
         spec_regs : dict
             Mapping from the names of the subsets to the subsets expressed
             as `specutils.SpectralRegion` objects.
         """
-        return self.app.get_subsets(spectral_only=True)
+        return self.app.get_subsets(spectral_only=True, use_display_units=use_display_units)
 
     def x_limits(self, x_min=None, x_max=None):
         """Sets the limits of the x-axis

jdaviz/configs/specviz/plugins/viewers.py

@@ -361,8 +361,10 @@ def add_data(self, data, color=None, alpha=None, **layer_state):
         result = super().add_data(data, color, alpha, **layer_state)
 
         if reset_plot_axes:
-            self.state.x_display_unit = data.get_component(self.state.x_att.label).units
-            self.state.y_display_unit = data.get_component("flux").units
+            x_units = data.get_component(self.state.x_att.label).units
+            y_units = data.get_component("flux").units
+            self.state.x_display_unit = x_units if len(x_units) else None
+            self.state.y_display_unit = y_units if len(y_units) else None
             self.set_plot_axes()
 
         self._plot_uncertainties()
@@ -489,7 +491,8 @@ def _plot_uncertainties(self):
     def set_plot_axes(self):
         # Set axes labels for the spectrum viewer
         flux_unit_type = "Flux density"
-        x_unit = u.Unit(self.state.x_display_unit)
+        x_disp_unit = self.state.x_display_unit
+        x_unit = u.Unit(x_disp_unit) if x_disp_unit else u.dimensionless_unscaled
         if x_unit.is_equivalent(u.m):
             spectral_axis_unit_type = "Wavelength"
         elif x_unit.is_equivalent(u.Hz):

jdaviz/configs/specviz/tests/test_helper.py

@@ -293,17 +293,21 @@ def test_get_spectral_regions_unit_conversion(specviz_helper, spectrum1d):
     assert label_mouseover.icon == ''
 
     # Convert the wavelength axis to micron
-    new_spectral_axis = "micron"
-    spec_viewer.state.x_display_unit = new_spectral_axis
-    spec_viewer.set_plot_axes()
+    new_spectral_axis = "um"
+    specviz_helper.plugins['Unit Conversion'].spectral_unit = new_spectral_axis
 
     spec_viewer.apply_roi(XRangeROI(0.6, 0.7))
 
     # Retrieve the Subset
-    subsets = specviz_helper.get_spectral_regions()
+    subsets = specviz_helper.get_spectral_regions(use_display_units=False)
+    reg = subsets.get('Subset 1')
+    assert reg.lower.unit == u.Angstrom
+    assert reg.upper.unit == u.Angstrom
+
+    subsets = specviz_helper.get_spectral_regions(use_display_units=True)
     reg = subsets.get('Subset 1')
-    assert reg.lower.unit == u.micron
-    assert reg.upper.unit == u.micron
+    assert reg.lower.unit == u.um
+    assert reg.upper.unit == u.um
 
     # Coordinates info panel should show new unit
     label_mouseover._viewer_mouse_event(spec_viewer,

jdaviz/core/helpers.py

@@ -418,13 +418,20 @@ def _handle_display_units(data, use_display_units):
                     spectral_unit = self.app._get_display_unit('spectral')
                     if not spectral_unit:
                         return data
+                    if self.app.config == 'cubeviz' and spectral_unit == 'deg':
+                        # this happens before the correct axis is set for the spectrum-viewer
+                        # and would result in a unit-conversion error if attempting to convert
+                        # to the display units.  This should only ever be temporary during
+                        # app intialization.
+                        return data
                     flux_unit = self.app._get_display_unit('flux')
                     # TODO: any other attributes (meta, wcs, etc)?
                     # TODO: implement uncertainty.to upstream
+                    new_uncert = data.uncertainty.__class__(data.uncertainty.quantity.to(flux_unit)) if data.uncertainty is not None else None  # noqa
                     data = Spectrum1D(spectral_axis=data.spectral_axis.to(spectral_unit,
                                                                           u.spectral()),
                                       flux=data.flux.to(flux_unit),
-                                      uncertainty=data.uncertainty.__class__(data.uncertainty.quantity.to(flux_unit)))  # noqa
+                                      uncertainty=new_uncert)
                 else:  # pragma: nocover
                     raise NotImplementedError(f"converting {data.__class__.__name__} to display units is not supported")  # noqa
             return data

jdaviz/core/marks.py

@@ -14,7 +14,7 @@
 
 __all__ = ['OffscreenLinesMarks', 'BaseSpectrumVerticalLine', 'SpectralLine',
            'SliceIndicatorMarks', 'ShadowMixin', 'ShadowLine', 'ShadowLabelFixedY',
-           'PluginMark', 'PluginLine', 'PluginScatter',
+           'PluginMark', 'LinesAutoUnit', 'PluginLine', 'PluginScatter',
            'LineAnalysisContinuum', 'LineAnalysisContinuumCenter',
            'LineAnalysisContinuumLeft', 'LineAnalysisContinuumRight',
            'LineUncertainties', 'ScatterMask', 'SelectedSpaxel', 'MarkersMark']

jdaviz/core/template_mixin.py

@@ -28,6 +28,7 @@
 
 
 __all__ = ['show_widget', 'TemplateMixin', 'PluginTemplateMixin',
+           'ViewerPropertiesMixin',
            'BasePluginComponent', 'SelectPluginComponent', 'UnitSelectPluginComponent',
            'PluginSubcomponent',
            'SubsetSelect', 'SpatialSubsetSelectMixin', 'SpectralSubsetSelectMixin',