refactor unit conversion messaging (#3192)

* refactor/simplify messaging and logic in unit conversion
* generalized solution in place of #3185
* generalized solution in place of #3177
* update slice plugin to handle new messaging order
* Revert "skip test in parsers" This reverts commit aa0d384.
* optimize handle_attribute_display_unit by caching image layers

CHANGES.rst

@@ -6,7 +6,7 @@ New Features
 
 - Added flux/surface brightness translation and surface brightness
   unit conversion in Cubeviz and Specviz. [#2781, #2940, #3088, #3111, #3113, #3129,
-  #3139, #3149, #3155, #3178, #3185, #3187, #3190, #3156, #3200]
+  #3139, #3149, #3155, #3178, #3185, #3187, #3190, #3156, #3200, #3192]
 
 - Plugin tray is now open by default. [#2892]
 

jdaviz/app.py

@@ -1345,8 +1345,7 @@ def _get_display_unit(self, axis):
                 raise ValueError(f"could not find units for axis='{axis}'")
         uc = self._jdaviz_helper.plugins.get('Unit Conversion')._obj
         if axis == 'spectral_y':
-            # translate options from uc.spectral_y_type to the prefix used in uc.??_unit_selected
-            axis = {'Surface Brightness': 'sb', 'Flux': 'flux'}[uc.spectral_y_type_selected]
+            return uc.spectral_y_unit
         try:
             return getattr(uc, f'{axis}_unit_selected')
         except AttributeError:

jdaviz/configs/cubeviz/plugins/mixins.py

@@ -89,6 +89,12 @@ def slice_values(self):
         take_inds = [2, 1, 0]
         take_inds.remove(self.slice_index)
         converted_axis = np.array([])
+
+        # Retrieve display units
+        slice_display_units = self.jdaviz_app._get_display_unit(
+            self.slice_display_unit_name
+        )
+
         for layer in self.layers:
             world_comp_ids = layer.layer.data.world_component_ids
 
@@ -100,11 +106,6 @@ def slice_values(self):
                 # Case where 2D image is loaded in image viewer
                 continue
 
-            # Retrieve display units
-            slice_display_units = self.jdaviz_app._get_display_unit(
-                self.slice_display_unit_name
-            )
-
             try:
                 # Retrieve layer data and units using the slice index of the world components ids
                 data_comp = layer.layer.data.get_component(world_comp_ids[self.slice_index])

jdaviz/configs/cubeviz/plugins/parsers.py

@@ -611,8 +611,9 @@ def convert_spectrum1d_from_flux_to_flux_per_pixel(spectrum):
     # and uncerts, if present
     uncerts = getattr(spectrum, 'uncertainty')
     if uncerts is not None:
-        old_uncerts = uncerts.represent_as(StdDevUncertainty)  # enforce common uncert type.
-        uncerts = old_uncerts.quantity / (u.pix * u.pix)
+        # enforce common uncert type.
+        uncerts = uncerts.represent_as(StdDevUncertainty)
+        uncerts = StdDevUncertainty(uncerts.quantity / (u.pix * u.pix))
 
     # create a new spectrum 1d with all the info from the input spectrum 1d,
     # and the flux / uncerts converted from flux to SB per square pixel

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

@@ -139,9 +139,9 @@ def _initialize_location(self, *args):
             if str(viewer.state.x_att) not in self.valid_slice_att_names:
                 # avoid setting value to degs, before x_att is changed to wavelength, for example
                 continue
-            # ensure the cache is reset (if previous attempts to initialize failed resulting in an
-            # empty list as the cache)
-            viewer._clear_cache('slice_values')
+            if self.app._get_display_unit(viewer.slice_display_unit_name) == '':
+                # viewer is not ready to retrieve slice_values in display units
+                continue
             slice_values = viewer.slice_values
             if len(slice_values):
                 self.value = slice_values[int(len(slice_values)/2)]
@@ -229,22 +229,30 @@ def _on_select_slice_message(self, msg):
             self.value = msg.value
 
     def _on_global_display_unit_changed(self, msg):
-        if not self.app.config == 'cubeviz':
-            return
-
         if msg.axis != self.slice_display_unit_name:
             return
+        self._clear_cache()
         if not self.value_unit:
             self.value_unit = str(msg.unit)
             return
+        if not self._indicator_initialized:
+            self._initialize_location()
+            return
         prev_unit = u.Unit(self.value_unit)
         self.value_unit = str(msg.unit)
-        self._clear_cache()
-        self.value = (self.value * prev_unit).to_value(msg.unit, equivalencies=u.spectral())
+        self.value = self._convert_value_to_unit(self.value, prev_unit, msg.unit)
+
+    def _convert_value_to_unit(self, value, prev_unit, new_unit):
+        return (value * prev_unit).to_value(new_unit, equivalencies=u.spectral())
 
     def _clear_cache(self, *attrs):
         if not len(attrs):
             attrs = self._cached_properties
+        if len(attrs):
+            # most internally cached properties rely on
+            # viewer slice_values, so let's also clear those caches
+            for viewer in self.slice_selection_viewers:
+                viewer._clear_cache('slice_values')
         for attr in attrs:
             if attr in self.__dict__:
                 del self.__dict__[attr]

jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py

@@ -584,6 +584,7 @@ def test_spectral_extraction_unit_conv_one_spec(
     uc = cubeviz_helper.plugins["Unit Conversion"]
     assert uc.flux_unit == "Jy"
     uc.flux_unit.selected = "MJy"
+    assert spectrum_viewer.state.y_display_unit == "MJy"
     spec_extr_plugin = cubeviz_helper.plugins['Spectral Extraction']
     # Overwrite the one and only default extraction.
     collapsed = spec_extr_plugin.extract()

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

@@ -160,7 +160,6 @@ def test_spectrum3d_no_wcs_parse(cubeviz_helper):
     assert flux.units == 'nJy / pix2'
 
 
-@pytest.mark.skip(reason="unskip after 3192 merged")
 def test_spectrum1d_parse(spectrum1d, cubeviz_helper):
     cubeviz_helper.load_data(spectrum1d)
 

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

@@ -472,9 +472,9 @@ def _initialize_model_component(self, model_comp, comp_label, poly_order=None):
 
         # Need to set the units the first time we initialize a model component, after this
         # we listen for display unit changes
-        if (self._units is None or self._units == {} or 'x' not in self._units or
-                'y' not in self._units):
+        if self._units.get('x', '') == '':
             self._units['x'] = self.app._get_display_unit('spectral')
+        if self._units.get('y', '') == '':
             if self.cube_fit:
                 self._units['y'] = self.app._get_display_unit('sb')
             else:
@@ -537,7 +537,7 @@ def _initialize_model_component(self, model_comp, comp_label, poly_order=None):
 
         # equivs for spectral density and flux<>flux/pix2. revisit
         # when generalizing plugin UC equivs.
-        equivs = _eqv_flux_to_sb_pixel() + [u.spectral_density(init_x)]
+        equivs = _eqv_flux_to_sb_pixel() + u.spectral_density(init_x)
         init_y = init_y.to(self._units['y'], equivs)
 
         initialized_model = initialize(

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

@@ -93,6 +93,10 @@ def test_parameter_retrieval(cubeviz_helper, spectral_cube_wcs):
     # even though the spectral y axis is in 'flux' by default
     plugin.cube_fit = True
 
+    assert cubeviz_helper.app._get_display_unit('spectral') == wav_unit
+    assert cubeviz_helper.app._get_display_unit('spectral_y') == flux_unit
+    assert cubeviz_helper.app._get_display_unit('sb') == sb_unit
+
     plugin.create_model_component("Linear1D", "L")
     with warnings.catch_warnings():
         warnings.filterwarnings('ignore', message='Model is linear in parameters.*')

jdaviz/configs/default/plugins/viewers.py

@@ -768,31 +768,29 @@ def set_plot_axes(self):
         # default to the catchall 'flux density' label
         flux_unit_type = None
 
-        if solid_angle_unit is not None:
-
-            for un in locally_defined_flux_units:
-                locally_defined_sb_unit = un / solid_angle_unit
-
-                # create an equivalency for each flux unit for flux <> flux/pix2.
-                # for similar reasons to the 'untranslatable units' issue, custom
-                # equivs. can't be combined, so a workaround is creating an eqiv
-                # for each flux that may need an additional equiv.
-                angle_to_pixel_equiv = _eqv_sb_per_pixel_to_per_angle(un)
-
-                if y_unit.is_equivalent(locally_defined_sb_unit, angle_to_pixel_equiv):
-                    flux_unit_type = "Surface Brightness"
-                elif y_unit.is_equivalent(un):
-                    flux_unit_type = 'Flux'
-                elif y_unit.is_equivalent(u.electron / u.s) or y_unit.physical_type == 'dimensionless':  # noqa
-                    # electron / s or 'dimensionless_unscaled' should be labeled counts
-                    flux_unit_type = "Counts"
-                elif y_unit.is_equivalent(u.W):
-                    flux_unit_type = "Luminosity"
-                if flux_unit_type is not None:
-                    # if we determined a label, stop checking
-                    break
-
-        if flux_unit_type is None:
+        for un in locally_defined_flux_units:
+            locally_defined_sb_unit = un / solid_angle_unit if solid_angle_unit is not None else None  # noqa
+
+            # create an equivalency for each flux unit for flux <> flux/pix2.
+            # for similar reasons to the 'untranslatable units' issue, custom
+            # equivs. can't be combined, so a workaround is creating an eqiv
+            # for each flux that may need an additional equiv.
+            angle_to_pixel_equiv = _eqv_sb_per_pixel_to_per_angle(un)
+
+            if (locally_defined_sb_unit is not None
+                    and y_unit.is_equivalent(locally_defined_sb_unit, angle_to_pixel_equiv)):
+                flux_unit_type = "Surface Brightness"
+            elif y_unit.is_equivalent(un):
+                flux_unit_type = 'Flux'
+            elif y_unit.is_equivalent(u.electron / u.s) or y_unit.physical_type == 'dimensionless':  # noqa
+                # electron / s or 'dimensionless_unscaled' should be labeled counts
+                flux_unit_type = "Counts"
+            elif y_unit.is_equivalent(u.W):
+                flux_unit_type = "Luminosity"
+            if flux_unit_type is not None:
+                # if we determined a label, stop checking
+                break
+        else:
             # default to Flux Density for flux density or uncaught types
             flux_unit_type = "Flux density"
 

jdaviz/configs/specviz/plugins/parsers.py

@@ -11,7 +11,6 @@
 from jdaviz.core.events import SnackbarMessage
 from jdaviz.core.registries import data_parser_registry
 from jdaviz.utils import standardize_metadata, download_uri_to_path
-from jdaviz.core.validunits import check_if_unit_is_per_solid_angle
 
 
 __all__ = ["specviz_spectrum1d_parser"]
@@ -160,15 +159,6 @@ def specviz_spectrum1d_parser(app, data, data_label=None, format=None, show_in_v
             # Make metadata layout conform with other viz.
             spec.meta = standardize_metadata(spec.meta)
 
-            # If this is the first loaded data, we want to set spectral y unit type to Flux or
-            # Surface Brightness as appropriate
-            if len(app.data_collection) == 0 and "Unit Conversion" in app._jdaviz_helper.plugins:
-                uc = app._jdaviz_helper.plugins["Unit Conversion"]
-                if check_if_unit_is_per_solid_angle(flux_units):
-                    uc._obj.spectral_y_type = "Surface Brightness"
-                else:
-                    uc._obj.spectral_y_type = "Flux"
-
             app.add_data(spec, data_label[i])
 
             # handle display, with the SpectrumList special case in mind.

jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py

@@ -100,7 +100,7 @@ def test_conv_no_data(specviz_helper, spectrum1d):
     # spectrum not load is in Flux units, sb_unit and flux_unit
     # should be enabled, spectral_y_type should not be
     plg = specviz_helper.plugins["Unit Conversion"]
-    with pytest.raises(ValueError, match="no valid unit choices"):
+    with pytest.raises(ValueError, match="could not find match in valid x display units"):
         plg.spectral_unit = "micron"
     assert len(specviz_helper.app.data_collection) == 0
 
@@ -290,11 +290,17 @@ def test_contour_unit_conversion(cubeviz_helper, spectrum1d_cube_fluxunit_jy_per
     # Make sure that the contour values get updated
     po_plg.contour_visible = True
 
+    assert uc_plg.spectral_y_type == 'Flux'
+    assert uc_plg.flux_unit == 'Jy'
+    assert uc_plg.sb_unit == "Jy / sr"
+    assert cubeviz_helper.viewers['flux-viewer']._obj.layers[0].state.attribute_display_unit == "Jy / sr"  # noqa
     assert np.allclose(po_plg.contour_max.value, 199)
 
-    uc_plg._obj.spectral_y_type_selected = 'Surface Brightness'
+    uc_plg.spectral_y_type = 'Surface Brightness'
     uc_plg.flux_unit = 'MJy'
 
+    assert uc_plg.sb_unit == "MJy / sr"
+    assert cubeviz_helper.viewers['flux-viewer']._obj.layers[0].state.attribute_display_unit == "MJy / sr"  # noqa
     assert np.allclose(po_plg.contour_max.value, 1.99e-4)