Update messaging from unit conversion plugin (#3155)

* change GlobalDisplayUnitChanged axis to spectral_y for clarity

* change messaging

* .

* broadcast 3 messages

* update spectral extraction plugin to listen to unit change messages

* add args

* some changes in aperture photometry to avoid unnecessary unit conversions

* revert changes to aperture phot

* code style and change log

* .

* update_uc_messaging

* .

* removed unused var

* remove config check

* fix model fitting

* update function selection

* fix model fitting

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]
+  unit conversion in Cubeviz and Specviz. [#2781, #2940, #3088, #3111, #3113, #3129, #3155]
 
 - Plugin tray is now open by default. [#2892]
 
@@ -133,6 +133,7 @@ Cubeviz
 
 - No longer incorrectly swap RA and Dec axes when loading Spectrum1D objects. [#3133]
 
+
 Imviz
 ^^^^^
 

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,31 @@ 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
+        elif msg.axis == 'flux':
+            self.flux_unit = str(msg.unit)
+        elif msg.axis == 'sb':
+            self.sb_unit = str(msg.unit)
+        else:
+            return
+
+        # and set results_units, which depends on function selected
+        self._update_units_on_function_selection()
+
     @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,12 @@ 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)
+        if msg.axis == 'spectral_y':
+            axis = 'y'
+        elif msg.axis == 'spectral':
+            axis = 'x'
+        else:
+            return
 
         # update internal tracking of current units
         self._units[axis] = str(msg.unit)

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

@@ -120,9 +120,18 @@ 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)
+
+        # even if data loaded is in 'flux' it can be represented as a
+        # per-pixel sb unit, so all cube data will be 'sb' (cubeviz)
+        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,40 +230,38 @@ 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))
+
+        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 flux_or_sb in untranslatable_units:
+        if spectral_y in untranslatable_units:
             self.can_translate = False
         else:
             self.can_translate = True
 
-        yunit = _valid_glue_display_unit(flux_or_sb, self.spectrum_viewer, 'y')
+        yunit = _valid_glue_display_unit(spectral_y, self.spectrum_viewer, 'y')
 
         # update spectrum viewer with new y display unit
         if self.spectrum_viewer.state.y_display_unit != yunit:
             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", spectral_y, 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 +296,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 +318,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

jdaviz/core/marks.py

@@ -127,7 +127,7 @@ def _on_global_display_unit_changed(self, msg):
         if not self.auto_update_units:
             return
         if self.viewer.__class__.__name__ in ['SpecvizProfileView', 'CubevizProfileView']:
-            axis_map = {'spectral': 'x', 'flux': 'y'}
+            axis_map = {'spectral': 'x', 'spectral_y': 'y'}
         elif self.viewer.__class__.__name__ == 'MosvizProfile2DView':
             axis_map = {'spectral': 'x'}
         else:

jdaviz/core/template_mixin.py

@@ -3547,7 +3547,7 @@ def _dc_to_dict(data):
         self._clear_cache(*self._cached_properties)
 
     def _on_global_display_unit_changed(self, msg=None):
-        if msg.axis in ('spectral', 'flux'):
+        if msg.axis in ('spectral', 'spectral_y'):
             self._clear_cache('selected_spectrum')