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)