Untranslatable unit handling (#3139)

* re-add untranslatable units, add translation equivalencies

* readd untranslatable units to app defined units

* fix rebase, enable untranslatable units, testing and coords needs work

* fix styling

* resolve spectrum-viewer mouseover, image-viewers need work

* generalize flux_conversion, enable untranslatable units, temporarily us mouseover global messages

* change log entry, update test_utils.py tests

* increase test coverage

* seperate flux conversion and transitive units, remove duplicate units, test coverage

* remove duplicate/reversed units

* implement commented changes

* update tests

* address review comments, begin work on spectral extraction issue

* resolve failing tests and spectral extraction issue

CHANGES.rst

@@ -5,7 +5,7 @@ New Features
 ------------
 
 - Added flux/surface brightness translation and surface brightness
-  unit conversion in Cubeviz and Specviz. [#2781, #2940, #3088, #3111, #3113, #3129, #3155]
+  unit conversion in Cubeviz and Specviz. [#2781, #2940, #3088, #3111, #3113, #3129, #3139, #3155]
 
 - Plugin tray is now open by default. [#2892]
 

jdaviz/app.py

@@ -75,18 +75,16 @@ def equivalent_units(self, data, cid, units):
                 include_prefix_units=True, equivalencies=eqv)))
                 + [
                     'Jy', 'mJy', 'uJy', 'MJy',
-                    'W / (m2 Hz)', 'W / (Hz m2)',  # Order is different in astropy v5.3
-                    'eV / (s m2 Hz)', 'eV / (Hz s m2)',
-                    'erg / (s cm2 Angstrom)', 'erg / (s cm2 Angstrom)',
-                    'erg / (s cm2 Hz)', 'erg / (Hz s cm2)',
-                    'ph / (Angstrom s cm2)',
-                    'ph / (Hz s cm2)', 'ph / (Hz s cm2)'
+                    'W / (Hz m2)', 'eV / (Hz s m2)',
+                    'erg / (Hz s cm2)', 'erg / (Angstrom s cm2)',
+                    'ph / (Angstrom s cm2)', 'ph / (Hz s cm2)'
                 ]
                 + [
                     'Jy / sr', 'mJy / sr', 'uJy / sr', 'MJy / sr',
-                    'W / (Hz sr m2)',
-                    'eV / (Hz s sr m2)',
-                    'erg / (s sr cm2)',
+                    'W / (Hz sr m2)', 'eV / (Hz s sr m2)',
+                    'erg / (s sr cm2)', 'erg / (Hz s sr cm2)',
+                    'erg / (Angstrom s sr cm2)',
+                    'ph / (Angstrom s sr cm2)', 'ph / (Hz s sr cm2)'
                 ])
         else:  # spectral axis
             # prefer Hz over Bq and um over micron
@@ -108,7 +106,7 @@ def to_unit(self, data, cid, values, original_units, target_units):
             except RuntimeError:
                 data = data.get_object(cls=NDDataArray)
                 spec = Spectrum1D(flux=data.data * u.Unit(original_units))
-            return flux_conversion(spec, values, original_units, target_units)
+            return flux_conversion(values, original_units, target_units, spec)
         else:  # spectral axis
             return spectral_axis_conversion(values, original_units, target_units)
 

jdaviz/configs/imviz/plugins/coords_info/coords_info.py

@@ -17,7 +17,7 @@
 from jdaviz.core.marks import PluginScatter, PluginLine
 from jdaviz.core.registries import tool_registry
 from jdaviz.core.template_mixin import TemplateMixin, DatasetSelectMixin
-from jdaviz.utils import _eqv_pixar_sr, _convert_surface_brightness_units
+from jdaviz.utils import flux_conversion, _eqv_pixar_sr
 
 __all__ = ['CoordsInfo']
 
@@ -482,11 +482,21 @@ def _image_viewer_update(self, viewer, x, y):
                 value = self._get_cube_value(
                     image, arr, x, y, viewer
                 )
-                if self.image_unit is not None and self.image_unit.is_equivalent(unit):
-                    value = _convert_surface_brightness_units(
-                        value, unit, self.image_unit
-                    )
-                    unit = self.image_unit
+                if self.image_unit is not None:
+                    if 'PIXAR_SR' in self.app.data_collection[0].meta:
+                        # Need current slice value and associated unit to use to compute
+                        # spectral density equivalencies that enable Flux to Flux conversions.
+                        # This is needed for units that are not directly convertible/translatable.
+                        slice = viewer.slice_value * u.Unit(self.app._get_display_unit('spectral'))
+
+                        value = flux_conversion(value, unit, self.image_unit,
+                                                eqv=_eqv_pixar_sr(self.app.data_collection[0].meta['PIXAR_SR']),  # noqa: E501
+                                                slice=slice)
+                        unit = self.image_unit
+
+                    elif self.image_unit.is_equivalent(unit):
+                        value = (value * u.Unit(unit)).to_value(u.Unit(self.image_unit))
+                        unit = self.image_unit
 
                 if associated_dq_layers is not None:
                     associated_dq_layer = associated_dq_layers[0]
@@ -590,8 +600,7 @@ def _copy_axes_to_spectral():
                 # temporarily here, may be removed after upstream units handling
                 # or will be generalized for any sb <-> flux
                 if '_pixel_scale_factor' in sp.meta:
-                    eqv = u.spectral_density(sp.spectral_axis) + _eqv_pixar_sr(sp.meta['_pixel_scale_factor'])  # noqa
-                    disp_flux = sp.flux.to_value(viewer.state.y_display_unit, eqv)
+                    disp_flux = flux_conversion(sp.flux.value, sp.flux.unit, viewer.state.y_display_unit, spec=sp)  # noqa: E501
                 else:
                     disp_flux = sp.flux.to_value(viewer.state.y_display_unit,
                                                  u.spectral_density(sp.spectral_axis))

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

@@ -9,7 +9,6 @@
 from jdaviz.core.validunits import (create_spectral_equivalencies_list,
                                     create_flux_equivalencies_list,
                                     check_if_unit_is_per_solid_angle,
-                                    units_to_strings,
                                     create_angle_equivalencies_list)
 
 __all__ = ['UnitConversion']
@@ -69,7 +68,6 @@ class UnitConversion(PluginTemplateMixin):
     flux_or_sb_items = List().tag(sync=True)
     flux_or_sb_selected = Unicode().tag(sync=True)
 
-    can_translate = Bool(True).tag(sync=True)
     # This is used a warning message if False. This can be changed from
     # bool to unicode when we eventually handle inputing this value if it
     # doesn't exist in the FITS header
@@ -170,9 +168,8 @@ def _on_glue_y_display_unit_changed(self, y_unit_str):
         # if the y-axis is set to surface brightness,
         # untranslatable units need to be removed from the flux choices
         if check_if_unit_is_per_solid_angle(y_unit_str):
-            updated_flux_choices = list(set(create_flux_equivalencies_list(y_unit * u.sr, x_unit))
-                                        - set(units_to_strings(self._untranslatable_units)))
-            self.flux_unit.choices = updated_flux_choices
+            flux_choices = create_flux_equivalencies_list(y_unit * u.sr, x_unit)
+            self.flux_unit.choices = flux_choices
 
         # sets the angle unit drop down and the surface brightness read-only text
         if self.app.data_collection[0]:
@@ -183,6 +180,19 @@ def _on_glue_y_display_unit_changed(self, y_unit_str):
                 self.flux_unit.selected,
                 self.angle_unit.selected
             )
+            if self.angle_unit.selected == 'pix':
+                mouseover_unit = self.flux_unit.selected
+            else:
+                mouseover_unit = self.sb_unit_selected
+            self.hub.broadcast(GlobalDisplayUnitChanged("sb", mouseover_unit, sender=self))
+
+        else:
+            # if cube was loaded in flux units, we still need to broadcast
+            # a 'sb' message for mouseover info. this should be removed when
+            # unit change messaging is improved and is a temporary fix
+            self.hub.broadcast(GlobalDisplayUnitChanged('sb',
+                                                        self.flux_unit.selected,
+                                                        sender=self))
 
             if not self.flux_unit.selected:
                 y_display_unit = self.spectrum_viewer.state.y_display_unit
@@ -242,15 +252,6 @@ def _on_flux_unit_changed(self, msg):
 
         spectral_y = sb_unit if self.flux_or_sb == 'Surface Brightness' else flux_unit
 
-        untranslatable_units = self._untranslatable_units
-        # disable translator if flux unit is untranslatable,
-        # still can convert flux units, this just disables flux
-        # to surface brightness translation for units in list.
-        if spectral_y in untranslatable_units:
-            self.can_translate = False
-        else:
-            self.can_translate = True
-
         yunit = _valid_glue_display_unit(spectral_y, self.spectrum_viewer, 'y')
 
         # update spectrum viewer with new y display unit
@@ -280,18 +281,6 @@ def _translate(self, flux_or_sb=None):
         if self.app.config == 'specviz':
             return
 
-        # we want to raise an error if a user tries to translate with an
-        # untranslated Flux unit using the API
-        untranslatable_units = units_to_strings(self._untranslatable_units)
-
-        if hasattr(self, 'flux_unit'):
-            if ((self.flux_unit.selected in untranslatable_units)
-                    and (flux_or_sb == 'Surface Brightness')):
-                raise ValueError(
-                    "Selected flux unit is not translatable. Please choose a flux unit "
-                    f"that is not in the following list: {untranslatable_units}."
-                )
-
         if self.spectrum_viewer.state.y_display_unit:
             spec_units = u.Unit(self.spectrum_viewer.state.y_display_unit)
         else:
@@ -325,17 +314,6 @@ def _translate(self, flux_or_sb=None):
                                                     sender=self))
         self.spectrum_viewer.reset_limits()
 
-    @property
-    def _untranslatable_units(self):
-        return [
-            u.erg / (u.s * u.cm**2),
-            u.erg / (u.s * u.cm**2 * u.Angstrom),
-            u.erg / (u.s * u.cm**2 * u.Hz),
-            u.ph / (u.Angstrom * u.s * u.cm**2),
-            u.ph / (u.s * u.cm**2 * u.Hz),
-            u.ST, u.bol
-        ]
-
     def _append_angle_correctly(self, flux_unit, angle_unit):
         if angle_unit not in ['pix', 'sr']:
             self.sb_unit_selected = flux_unit

jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue

@@ -74,9 +74,7 @@
         :class="api_hints_enabled ? 'api-hint' : null"
         hint="Select the y-axis physical type for the spectrum-viewer."
         persistent-hint
-        :disabled="!can_translate"
       ></v-select>
-      <span v-if="!can_translate">Translation is not available due to current unit selection.</span>
     </v-row>
 
     <v-alert type="warning" v-if="!pixar_sr_exists && config == 'cubeviz'">

jdaviz/core/helpers.py

@@ -487,20 +487,20 @@ def _handle_display_units(self, data, use_display_units=True):
                         # if not specified as NDUncertainty, assume stddev:
                         new_uncert = uncertainty
                     if ('_pixel_scale_factor' in data.meta):
-                        new_uncert_converted = flux_conversion(data, new_uncert.quantity.value,
-                                                               new_uncert.unit, y_unit)
+                        new_uncert_converted = flux_conversion(new_uncert.quantity.value,
+                                                               new_uncert.unit, y_unit, spec=data)
                         new_uncert = StdDevUncertainty(new_uncert_converted, unit=y_unit)
                     else:
                         new_uncert = StdDevUncertainty(new_uncert, unit=data.flux.unit)
 
                 else:
                     new_uncert = None
                 if ('_pixel_scale_factor' in data.meta):
-                    new_y = flux_conversion(data, data.flux.value, data.flux.unit,
-                                            y_unit) * u.Unit(y_unit)
+                    new_y = flux_conversion(data.flux.value, data.flux.unit,
+                                            y_unit, data) * u.Unit(y_unit)
                 else:
-                    new_y = flux_conversion(data, data.flux.value, data.flux.unit,
-                                            data.flux.unit) * u.Unit(data.flux.unit)
+                    new_y = flux_conversion(data.flux.value, data.flux.unit,
+                                            data.flux.unit, spec=data) * u.Unit(data.flux.unit)
                 new_spec = (spectral_axis_conversion(data.spectral_axis.value,
                                                      data.spectral_axis.unit,
                                                      spectral_unit)

jdaviz/core/validunits.py

@@ -73,12 +73,9 @@ def create_flux_equivalencies_list(flux_unit, spectral_axis_unit):
 
     # Get local flux units.
     locally_defined_flux_units = ['Jy', 'mJy', 'uJy', 'MJy',
-                                  'W / (Hz m2)',
-                                  'eV / (s m2 Hz)',
-                                  'erg / (s cm2 Hz)',
-                                  'erg / (s cm2 Angstrom)',
-                                  'ph / (Angstrom s cm2)',
-                                  'ph / (Hz s cm2)',
+                                  'W / (Hz m2)', 'eV / (Hz s m2)',
+                                  'erg / (s cm2 Angstrom)', 'erg / (Hz s cm2)',
+                                  'ph / (Angstrom s cm2)', 'ph / (Hz s cm2)'
                                   ]
     local_units = [u.Unit(unit) for unit in locally_defined_flux_units]
 

jdaviz/tests/test_app.py

@@ -227,8 +227,8 @@ def test_to_unit(cubeviz_helper):
     original_units = u.MJy / u.sr
     target_units = u.MJy
 
-    value = flux_conversion(data.get_object(cls=Spectrum1D),
-                            values, original_units, target_units)
+    value = flux_conversion(values, original_units,
+                            target_units, data.get_object(cls=Spectrum1D))
 
     # will be a uniform array since not wavelength dependent
     # so test first value in array
@@ -240,8 +240,8 @@ def test_to_unit(cubeviz_helper):
     original_units = u.MJy
     target_units = u.erg / u.cm**2 / u.s / u.AA
 
-    new_values = flux_conversion(data.get_object(cls=Spectrum1D), values,
-                                 original_units, target_units)
+    new_values = flux_conversion(values, original_units,
+                                 target_units, data.get_object(cls=Spectrum1D))
 
     assert np.allclose(new_values,
                        (values * original_units)
@@ -255,8 +255,8 @@ def test_to_unit(cubeviz_helper):
     original_units = u.MJy
     target_units = u.erg / u.cm**2 / u.s / u.AA
 
-    new_values = flux_conversion(data.get_object(cls=Spectrum1D), values,
-                                 original_units, target_units)
+    new_values = flux_conversion(values, original_units,
+                                 target_units, data.get_object(cls=Spectrum1D))
 
     # In this case we do a regular spectral density conversion, but using the
     # first value in the spectral axis for the equivalency