update unit conversion message responses in plugins, add tests

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

@@ -109,6 +109,8 @@ class SpectralExtraction(PluginTemplateMixin, ApertureSubsetSelectMixin,
 
     results_units = Unicode().tag(sync=True)
     spectrum_y_units = Unicode().tag(sync=True)
+    flux_unit = Unicode().tag(sync=True)
+    sb_unit = Unicode().tag(sync=True)
 
     aperture_method_items = List().tag(sync=True)
     aperture_method_selected = Unicode('Center').tag(sync=True)
@@ -178,7 +180,7 @@ def __init__(self, *args, **kwargs):
         self.session.hub.subscribe(self, SliceValueUpdatedMessage,
                                    handler=self._on_slice_changed)
         self.hub.subscribe(self, GlobalDisplayUnitChanged,
-                           handler=self._update_results_units)
+                           handler=self._on_gloabl_display_unit_changed)
 
         self._update_disabled_msg()
 
@@ -313,13 +315,27 @@ def _update_mark_scale(self, *args):
         else:
             self.background.scale_factor = self.slice_spectral_value/self.reference_spectral_value
 
+    def _on_gloabl_display_unit_changed(self, msg={}):
+
+        if msg.axis == 'spectral_y':
+            self.spectrum_y_units = str(msg.unit)
+
+        # a 'flux' and 'sb' message should be recieved back to back from
+        # the unit conversion plugin, so don't need to sync them immediatley
+        # within each message recieved
+        if msg.axis == 'flux':
+            self.flux_unit = str(msg.unit)
+        if msg.axis == 'sb':
+            self.sb_unit = str(msg.unit)
+
     @observe('function_selected')
-    def _update_results_units(self, msg={}):
-        self.spectrum_y_units = str(self.app._get_display_unit('spectral_y'))
+    def _update_units_on_function_selection(self, *args):
+
         if self.function_selected.lower() == 'sum':
-            self.results_units = str(self.app._get_display_unit('flux'))
+            self.results_units = self.flux_unit
         else:
-            self.results_units = str(self.app._get_display_unit('sb'))
+            self.results_units = self.sb_unit
+
 
     @observe('function_selected', 'aperture_method_selected')
     def _update_aperture_method_on_function_change(self, *args):

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

@@ -528,7 +528,7 @@ def _check_model_component_compat(self, axes=['x', 'y'], display_units=None):
         self._check_model_equation_invalid()
 
     def _on_global_display_unit_changed(self, msg):
-        axis = {'spectral': 'x', 'flux': 'y'}.get(msg.axis)
+        axis = {'spectral': 'x', 'spectral_y': 'y'}.get(msg.axis)
 
         # update internal tracking of current units
         self._units[axis] = str(msg.unit)

jdaviz/configs/imviz/plugins/aper_phot_simple/aper_phot_simple.py

@@ -150,7 +150,7 @@ def _on_dataset_selected_changed(self, event={}):
         else:
             self.is_cube = False
 
-    def _on_display_units_changed(self, event={}):
+    def _on_display_units_changed(self, msg={}):
 
         """
         Handle change of display units from Unit Conversion plugin (for now,
@@ -160,76 +160,105 @@ def _on_display_units_changed(self, event={}):
         'calculate' is pressed again.
         """
 
+        # only concerned with unit changes in cubeviz, for now
         if self.config == 'cubeviz':
 
-            # get previously selected display units
-            prev_display_flux_or_sb_unit = self.display_flux_or_sb_unit
-            prev_flux_scale_unit = self.flux_scaling_display_unit
-
-            # update display unit traitlets to new selection
-            self._set_display_unit_of_selected_dataset()
-
-            # convert the previous background and flux scaling values to new unit so
-            # re-calculating photometry with the current selections will produce
-            # the previous output with the new unit.
-            if prev_display_flux_or_sb_unit != '':
-
+            if msg.axis == 'flux':
+                # update the traitlet for the flux scaling display unit when
+                # the global flux display unit has been changed
+                # a 'flux' message will be immediatley followed by a 'sb'
+                # message upon unit change, so the rest will be handled there
+
+                self.flux_scaling_display_unit = msg.unit
+
+                # get previously selected display unit
+                prev_flux_scale_unit = self.flux_scaling_display_unit
+
+                if prev_flux_scale_unit = != '':
+                    if self.flux_scaling is not None:
+                        prev_unit = u.Unit(prev_flux_scale_unit)
+                        new_unit = u.Unit(self.flux_scaling_display_unit)
+                        self._convert_flux_scaling_to_new_units(prev_unit, new_unit)
+
+            if msg.axis == 'sb':
+                # get previously selected display unit
+                prev_display_flux_or_sb_unit = self.display_flux_or_sb_unit
+
+                # update display unit traitlet to new selection
+                disp_unit = msg.unit
+                self._set_display_flux_or_sb_unit_traitlet(msg.unit)
+
                 # convert background to new unit
-                if self.background_value is not None:
+                if prev_display_flux_or_sb_unit != '':
+                    if self.background_value is not None:
+                        prev_unit = u.Unit(prev_display_flux_or_sb_unit)
+                        new_unit = u.Unit(self.display_flux_or_sb_unit)
+                        self._convert_background_to_new_units(prev_unit, new_unit)
 
-                    prev_unit = u.Unit(prev_display_flux_or_sb_unit)
-                    new_unit = u.Unit(self.display_flux_or_sb_unit)
+    def _set_display_flux_or_sb_unit_traitlet(self, disp_unit):
+        # in its own method rather than setting directly because there is some
+        # additional logic to handle pixel units, and can be called when units
+        # change or when dataset changes
 
-                    bg = self.background_value * prev_unit
-                    self.background_value = bg.to_value(
-                        new_unit, u.spectral_density(self._cube_wave))
+        # temporarily, until non-sr units are suppported, strip 'pix'
+        # from unit if it is a per-pixel sb unit
+        if 'pix' in disp_unit.bases:
+            disp_unit = u.Unit(image_unit) * u.pix
+            disp_unit = disp_unit.to_string()
 
-                # convert flux scaling to new unit
-                if self.flux_scaling is not None:
-                    prev_unit = u.Unit(prev_flux_scale_unit)
-                    new_unit = u.Unit(self.flux_scaling_display_unit)
+        self.display_flux_or_sb_unit = disp_unit
 
-                    fs = self.flux_scaling * prev_unit
-                    self.flux_scaling = fs.to_value(
-                        new_unit, u.spectral_density(self._cube_wave))
+    def _convert_background_to_new_units(self, prev_unit, new_unit):
 
-    def _set_display_unit_of_selected_dataset(self):
+        bg = self.background_value * prev_unit
+        self.background_value = bg.to_value(new_unit,
+                                            u.spectral_density(self._cube_wave))
 
-        """
-        Set the display_flux_or_sb_unit and flux_scaling_display_unit traitlets,
-        which depend on if the selected data set is flux or surface brightness,
-        and the corresponding global display unit for either flux or
-        surface brightness.
-        """
+    def _convert_flux_scaling_to_new_units(self, prev_unit, new_unit):
+        fs = self.flux_scaling * prev_unit
 
-        if not self.dataset_selected or not self.aperture_selected:
-            self.display_flux_or_sb_unit = ''
-            self.flux_scaling_display_unit = ''
+        self.flux_scaling = fs.to_value( new_unit, u.spectral_density(self._cube_wave))
+
+    @observe('dataset_selected')
+    def _dataset_selected_changed(self, event={}):
+        if not hasattr(self, 'dataset'):
+            # plugin not fully initialized
+            return
+        if self.dataset.selected_dc_item is None:
+            return
+        if self.multiselect:
+            # defaults are applied within the loop if the auto-switches are enabled,
+            # but we still need to update the flux-scaling warning
+            self._multiselect_flux_scaling_warning()
             return
 
-        data = self.dataset.selected_dc_item
-        comp = data.get_component(data.main_components[0])
-        if comp.units:
-            # if data is something-per-solid-angle, its a SB unit and we should
-            # use the selected global display unit for SB
-            if check_if_unit_is_per_solid_angle(comp.units):
-                flux_or_sb = 'sb'
+        try:
+            defaults = self._get_defaults_from_metadata()
+            self.counts_factor = 0
+            self.pixel_area = defaults.get('pixel_area', 0)
+            self.flux_scaling = defaults.get('flux_scaling', 0)
+            if 'flux_scaling' in defaults:
+                self.flux_scaling_warning = ''
             else:
-                flux_or_sb = 'flux'
+                self.flux_scaling_warning = ('Could not determine flux scaling for '
+                                             f'{self.dataset.selected}, defaulting to zero.')
 
-            disp_unit = self.app._get_display_unit(flux_or_sb)
+        except Exception as e:
+            self.hub.broadcast(SnackbarMessage(
+                f"Failed to extract {self.dataset_selected}: {repr(e)}",
+                color='error', sender=self))
 
-            self.display_flux_or_sb_unit = disp_unit
+        # get correct display unit for newly selected dataset
+        if self.config == 'cubeviz':
+            # 'get_display_unit' should be used when we are not intercepting
+            # a message with the correct unit from the UC plugin
+            sb_unit = self.app._get_display_unit('sb')
+            self._set_display_flux_or_sb_unit_traitlet(sb_unit)
+            self.flux_scaling_display_unit = self.app._get_display_unit('flux')
 
-            # now get display unit for flux_scaling_display_unit. this unit will always
-            # be in flux, but it will not be derived from the global flux display unit
-            # note : need to generalize this for non-sr units eventually
-            fs_unit = u.Unit(disp_unit) * u.sr
-            self.flux_scaling_display_unit = fs_unit.to_string()
+        # auto-populate background, if applicable.
+        self._aperture_selected_changed()
 
-        else:
-            self.display_flux_or_sb_unit = ''
-            self.flux_scaling_display_unit = ''
 
     def _get_defaults_from_metadata(self, dataset=None):
         defaults = {}
@@ -306,43 +335,6 @@ def _singleselect_flux_scaling_warning(self, event={}):
             # disable warning once user changes value
             self.flux_scaling_warning = ''
 
-    @observe('dataset_selected')
-    def _dataset_selected_changed(self, event={}):
-        if not hasattr(self, 'dataset'):
-            # plugin not fully initialized
-            return
-        if self.dataset.selected_dc_item is None:
-            return
-        if self.multiselect:
-            # defaults are applied within the loop if the auto-switches are enabled,
-            # but we still need to update the flux-scaling warning
-            self._multiselect_flux_scaling_warning()
-            return
-
-        try:
-            defaults = self._get_defaults_from_metadata()
-            self.counts_factor = 0
-            self.pixel_area = defaults.get('pixel_area', 0)
-            self.flux_scaling = defaults.get('flux_scaling', 0)
-            if 'flux_scaling' in defaults:
-                self.flux_scaling_warning = ''
-            else:
-                self.flux_scaling_warning = ('Could not determine flux scaling for '
-                                             f'{self.dataset.selected}, defaulting to zero.')
-
-        except Exception as e:
-            self.hub.broadcast(SnackbarMessage(
-                f"Failed to extract {self.dataset_selected}: {repr(e)}",
-                color='error', sender=self))
-
-        # get correct display unit for newly selected dataset
-        if self.config == 'cubeviz':
-            # set display_flux_or_sb_unit and flux_scaling_display_unit
-            self._set_display_unit_of_selected_dataset()
-
-        # auto-populate background, if applicable.
-        self._aperture_selected_changed()
-
     def _on_subset_update(self, msg):
         if not self.dataset_selected or not self.aperture_selected:
             return

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

@@ -12,12 +12,13 @@
 from jdaviz.configs.mosviz.plugins.viewers import (MosvizImageView, MosvizProfileView,
                                                    MosvizProfile2DView)
 from jdaviz.configs.specviz.plugins.viewers import SpecvizProfileView
-from jdaviz.core.events import ViewerAddedMessage, GlobalDisplayUnitChanged
+from jdaviz.core.events import ViewerAddedMessage, GlobalDisplayUnitChanged, SnackbarMessage
 from jdaviz.core.helpers import data_has_valid_wcs
 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.core.validunits import check_if_unit_is_per_solid_angle
 
 __all__ = ['CoordsInfo']
 
@@ -120,9 +121,21 @@ def _on_viewer_added(self, msg):
         self._create_viewer_callbacks(self.app.get_viewer_by_id(msg.viewer_id))
 
     def _on_global_display_unit_changed(self, msg):
-        # eventually should observe change in flux OR angle
-        if msg.axis == "flux":
-            self.image_unit = u.Unit(msg.unit)
+
+        # only concerned with handling unit changes for mouseover in cubeviz,
+        # until implemented in other configs
+        if self.config == 'cubeviz':
+            # even if data loaded is in 'flux' it can be represented as a
+            # per-pixel sb unit, so all cube data will be 'sb'
+            if msg.axis == "sb":
+                image_unit = u.Unit(msg.unit)
+
+                # temporarily, until non-sr units are suppported, strip 'pix' from
+                # unit if it is a per-pixel unit
+                if 'pix' in image_unit.bases:
+                    image_unit = image_unit * u.pix
+
+                self.image_unit = u.Unit(image_unit)
 
     @property
     def marks(self):

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

@@ -230,21 +230,22 @@ def _on_flux_unit_changed(self, msg):
         if not self.flux_unit.choices and self.app.config == 'cubeviz':
             return
 
-        flux_or_sb = None
-
         # when the configuration is Specviz, translation is not currently supported.
         # If in Cubeviz, all spectra pass through Spectral Extraction plugin and will
         # have a scale factor assigned in the metadata, enabling translation.
         current_y_unit = self.spectrum_viewer.state.y_display_unit
 
-        # if the current y display unit is a surface brightness unit,
-        if self.angle_unit.selected and check_if_unit_is_per_solid_angle(current_y_unit):
-            flux_or_sb = self._append_angle_correctly(
-                         self.flux_unit.selected,
-                         self.angle_unit.selected
-            )
-        else:
-            flux_or_sb = self.flux_unit.selected
+        # various plugins are listening for changes in either flux or sb and 
+        # need to be able to filter messages accordingly, so broadcast both when
+        # flux unit is updated. if data was loaded in a flux unit (i.e MJy), it
+        # can be reperesented as a per-pixel surface brightness unit
+        flux_unit = self.flux_unit.selected
+        sb_unit = self._append_angle_correctly(flux_unit, self.angle_unit.selected)
+
+        self.hub.broadcast(GlobalDisplayUnitChanged("flux", flux_unit, sender=self))
+        self.hub.broadcast(GlobalDisplayUnitChanged("sb", sb_unit, sender=self))
+
+        flux_or_sb = sb_unit if check_if_unit_is_per_solid_angle(current_y_unit) else flux_unit
 
         untranslatable_units = self._untranslatable_units
         # disable translator if flux unit is untranslatable,
@@ -262,8 +263,10 @@ def _on_flux_unit_changed(self, msg):
             self.spectrum_viewer.state.y_display_unit = yunit
             self.spectrum_viewer.reset_limits()
 
-            # and broacast that there has been a change in flux
-            self.hub.broadcast(GlobalDisplayUnitChanged("flux", flux_or_sb, sender=self))
+            # and broacast that there has been a change in the spectral axis y unit
+            # to either a flux or surface brightness unit, for plugins that specifically
+            # care about this toggle selection
+            self.hub.broadcast(GlobalDisplayUnitChanged("spectral_y", flux_or_sb, sender=self))
 
         if not check_if_unit_is_per_solid_angle(self.spectrum_viewer.state.y_display_unit):
             self.flux_or_sb_selected = 'Flux'
@@ -298,10 +301,12 @@ def _translate(self, flux_or_sb=None):
             spec_units = u.Unit(self.spectrum_viewer.state.y_display_unit)
         else:
             return
+
         # on instantiation, we set determine flux choices and selection
         # after surface brightness
         if not self.flux_unit.choices:
             return
+
         # Surface Brightness -> Flux
         if check_if_unit_is_per_solid_angle(spec_units) and flux_or_sb == 'Flux':
             spec_units *= u.sr
@@ -318,7 +323,9 @@ def _translate(self, flux_or_sb=None):
         else:
             return
 
-        self.hub.broadcast(GlobalDisplayUnitChanged('flux',
+        # broadcast that there has been a change in the spectrum viewer y axis,
+        # if translation was completed
+        self.hub.broadcast(GlobalDisplayUnitChanged('spectral_y',
                                                     spec_units,
                                                     sender=self))
         self.spectrum_viewer.reset_limits()

jdaviz/core/events.py

@@ -408,7 +408,10 @@ def flip_horizontal(self):
 
 
 class GlobalDisplayUnitChanged(Message):
-    '''Message generated when the global app-wide display unit is changed'''
+    '''Message generated when the (x or y axis) unit of the spectrum viewer is
+    changed, which is used app-wide to inform display units that depend on the 
+    unit choice and flux<>sb toggle of the spectrum viewer.'''
+
     def __init__(self, axis, unit, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self._axis = axis