changing from switch to dropdown, updating logic for conversion

jdaviz/app.py

@@ -83,7 +83,7 @@ def equivalent_units(self, data, cid, units):
                     'erg / (s cm2 Angstrom)', 'erg / (Angstrom s cm2)',
                     'erg / (s cm2 Hz)', 'erg / (Hz s cm2)',
                     'ph / (s cm2 Angstrom)', 'ph / (Angstrom s cm2)',
-                    'ph / (s cm2 Hz)', 'ph / (Hz s cm2)'
+                    'ph / (Hz s cm2)', 'ph / (Hz s cm2)', 'bol', 'AB', 'ST'
                 ]
                 + [
                     'Jy / sr', 'mJy / sr', 'uJy / sr', 'MJy / sr',
@@ -112,50 +112,67 @@ def to_unit(self, data, cid, values, original_units, target_units):
             except RuntimeError:
                 eqv = []
             if len(values) == 2:
-                    # Need this for setting the y-limits
-                    spec_limits = [spec.spectral_axis[0].value, spec.spectral_axis[-1].value]
-                    eqv = u.spectral_density(spec_limits * spec.spectral_axis.unit)
-
-            elif '_pixel_scale_factor' in spec.meta:
-                if (u.sr in u.Unit(original_units).bases) and \
-                   (u.sr not in u.Unit(target_units).bases):
-                    # sb -> flux
-
-                    # to debug
-                    print('first if')
-                    print('values before')
-                    print(values)
+                # Need this for setting the y-limits
+                spec_limits = [spec.spectral_axis[0].value, spec.spectral_axis[-1].value]
+                eqv = u.spectral_density(spec_limits * spec.spectral_axis.unit)
 
-                    converted_values = values * spec.meta['_pixel_scale_factor']
-
-                    # to debug
-                    print(converted_values)
+            # Ensure a spectrum passed through Spectral Extraction plugin
+            elif '_pixel_scale_factor' in spec.meta:
+                # if spectrum data collection item is in Surface Brightness units
+                if u.sr in spec.unit.bases:
+                    # Data item in data collection does not update from conversion/translation.
+                    # App wide orginal data units are used for conversion, orginal_units and
+                    # target_units dicate the conversion to take place.
+                    if (u.sr in u.Unit(original_units).bases) and \
+                       (u.sr not in u.Unit(target_units).bases) and \
+                       (target_units not in ["AB", "bol", "ST"]):  # astropy supported equivalency
+                        # Surface Brightness -> Flux
+                        eqv = [(u.MJy / u.sr,
+                                u.MJy,
+                                lambda x: (x * spec.meta['_pixel_scale_factor']),
+                                lambda x: x)]
+
+                        # below is the generalized version
+                        '''
+                        eqv = [(u.Unit(original_units),
+                                u.Unit(target_units),
+                                lambda x: (x * spec.meta['_pixel_scale_factor']),
+                                lambda x: x)]
+                        '''
+                    else:
+                        # Flux -> Surface Brightness
+                        eqv = u.spectral_density(spec.spectral_axis)
+
+                # if spectrum data collection item is in Flux units
+                elif u.sr not in spec.unit.bases:
+                    # Data item in data collection does not update from conversion/translation.
+                    # App wide orginal data units are used for conversion, orginal_units and
+                    # target_units dicate the conversion to take place.
+                    if (u.sr not in u.Unit(original_units).bases) and \
+                       (u.sr in u.Unit(target_units).bases) and \
+                       (target_units not in ["AB", "bol", "ST"]):  # astropy supported equivalency
+                        # Flux -> Surface Brightness
+                        eqv = [(u.MJy,
+                                u.MJy / u.sr,
+                                lambda x: (x / spec.meta['_pixel_scale_factor']),
+                                lambda x: x)]
+
+                        # below is the generalized version
+                        '''
+                        eqv = [(u.Unit(original_units),
+                                u.Unit(target_units),
+                                lambda x: (x / spec.meta['_pixel_scale_factor']),
+                                lambda x: x)]
+                        '''
+                    else:
+                        # Surface Brightness -> Flux
+                        eqv = u.spectral_density(spec.spectral_axis)
 
-                    return converted_values
+                # flux cid, but neither Surface Brightness nor Flux units
                 else:
-                    # elif (u.sr in u.Unit(target_units).bases) and \
-                    # (u.sr not in u.Unit(original_units).bases):
-                    # flux -> sb
-
-                    # to debug
-                    print('second elif')
-                    print('values before')
-                    print(values)
-
-                    converted_values = values / spec.meta['_pixel_scale_factor']
-                    # to debug
-                    print(converted_values)
-                    return converted_values
-                # custom equivalency
-                '''
-                eqv = [(u.MJy / u.sr,
-                        u.MJy,
-                        lambda x: (x * spec.meta['_pixel_scale_factor']),
-                        lambda x: x)]
-                '''
-
+                    raise ValueError('flux cid, but not a flux nor surface brightness unit.')
             else:
-                    eqv = u.spectral_density(spec.spectral_axis)
+                eqv = u.spectral_density(spec.spectral_axis)
 
         else:  # spectral axis
             eqv = u.spectral()

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

@@ -6,8 +6,6 @@
 from astropy import units as u
 from astropy.nddata import NDDataArray, StdDevUncertainty
 from astropy.utils.exceptions import AstropyUserWarning
-from glue.core.roi import CircularROI
-from glue.core.edit_subset_mode import ReplaceMode
 from numpy.testing import assert_allclose, assert_array_equal
 from regions import (CirclePixelRegion, CircleAnnulusPixelRegion, EllipsePixelRegion,
                      RectanglePixelRegion, PixCoord)

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

@@ -127,7 +127,7 @@ def test_non_stddev_uncertainty(specviz_helper):
 
 
 def test_unit_translation(cubeviz_helper):
-    # custom cube so we have PIXAR_SR in metadata, and flux units = Jy/pix
+    # custom cube so PIXAR_SR is in metadata, and Flux units, and in MJy
     wcs_dict = {"CTYPE1": "WAVE-LOG", "CTYPE2": "DEC--TAN", "CTYPE3": "RA---TAN",
                 "CRVAL1": 4.622e-7, "CRVAL2": 27, "CRVAL3": 205,
                 "CDELT1": 8e-11, "CDELT2": 0.0001, "CDELT3": -0.0001,
@@ -150,31 +150,42 @@ def test_unit_translation(cubeviz_helper):
     # set so pixel scale factor != 1
     extract_plg.reference_spectral_value = 0.000001
 
-    # collapse to spectrum, now we can get pixel scale factor
+    # all spectra will pass through spectral extraction,
+    # this will store a scale factor for use in translations.
     collapsed_spec = extract_plg.collapse_to_spectrum()
 
+    # test that the scale factor was set
     assert collapsed_spec.meta['_pixel_scale_factor'] != 1
 
-    # store to test second time after calling translate_units
-    mjy_sr_data1 = collapsed_spec._data[0]
-
     uc_plg = cubeviz_helper.plugins['Unit Conversion']
+    # When the dropdown is displayed, this ensures the loaded
+    # data collection item units will be used for translations.
+    uc_plg._obj.show_translator = True
+
+    # to have access to display units
+    viewer_1d = cubeviz_helper.app.get_viewer(
+        cubeviz_helper._default_spectrum_viewer_reference_name)
+
+    # change global y-units from Flux -> Surface Brightness
+    uc_plg._obj.flux_or_sb_selected = 'Surface Brightness'
+    y_display_unit = u.Unit(viewer_1d.state.y_display_unit)
 
-    print(cubeviz_helper.app.data_collection[1])
-    data = cubeviz_helper.app.data_collection[1].data
-    spec = data.get_object(cls=Spectrum1D)
-    print("meta data", spec.meta['_pixel_scale_factor'])
+    # check if units translated
+    assert y_display_unit == u.MJy / u.sr
 
-    uc_plg._obj._translate()
+    # Surface Brightness conversion
+    uc_plg.flux_unit_selected = 'W / (Hz sr m2)'
+    assert y_display_unit == u.W / (u.m**2 * u.sr * u.Hz)
 
-    assert collapsed_spec._unit == u.MJy / u.sr
-    # some value in MJy/sr that we know the outcome after translation
-    assert np.allclose(collapsed_spec._data[0], 8.7516529e10)
+    # A second Surface Brightness conversion
+    uc_plg.flux_unit_selected = 'Jy / sr'
+    assert y_display_unit == u.Jy / u.sr
 
-    uc_plg._obj._translate()
+    # Translate back to Flux
+    uc_plg._obj.flux_or_sb_selected = 'Flux'
+    y_display_unit = u.Unit(viewer_1d.state.y_display_unit)
+    assert y_display_unit == u.Jy
 
-    # translating again returns the original units
-    assert collapsed_spec._unit == u.MJy
-    # returns to the original values
-    # which is a value in Jy/pix that we know the outcome after translation
-    assert np.allclose(collapsed_spec._data[0], mjy_sr_data1)
+    # test flux conversion
+    uc_plg.flux_unit_selected = 'ph / (Angstrom s cm2)'
+    assert y_display_unit == u.ph / (u.s * u.cm**2 * u.Angstrom)

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

@@ -4,8 +4,8 @@
 
 from jdaviz.core.events import GlobalDisplayUnitChanged
 from jdaviz.core.registries import tray_registry
-from jdaviz.core.template_mixin import (PluginTemplateMixin, UnitSelectPluginComponent, SelectPluginComponent,
-                                        PluginUserApi)
+from jdaviz.core.template_mixin import (PluginTemplateMixin, UnitSelectPluginComponent,
+                                        SelectPluginComponent, PluginUserApi)
 from jdaviz.core.validunits import (create_spectral_equivalencies_list,
                                     create_flux_equivalencies_list)
 
@@ -53,9 +53,7 @@ class UnitConversion(PluginTemplateMixin):
     flux_unit_items = List().tag(sync=True)
     flux_unit_selected = Unicode().tag(sync=True)
 
-    translate_unit = Bool(False).tag(sync=True)
     show_translator = Bool(False).tag(sync=True)
-
     flux_or_sb_items = List().tag(sync=True)
     flux_or_sb_selected = Unicode().tag(sync=True)
 
@@ -139,17 +137,21 @@ def _on_glue_y_display_unit_changed(self, y_unit):
     def translate_units(self, flux_or_sb_selected):
         spec_units = u.Unit(self.spectrum_viewer.state.y_display_unit)
         # Surface Brightness -> Flux
-        if u.sr in spec_units.bases and flux_or_sb_selected == 'Surface Brightness':
+        if u.sr in spec_units.bases and flux_or_sb_selected == 'Flux':
             spec_units *= u.sr
             # update display units
             self.spectrum_viewer.state.y_display_unit = str(spec_units)
 
         # Flux -> Surface Brightness
-        elif u.sr not in spec_units.bases and flux_or_sb_selected == 'Flux':
+        elif u.sr not in spec_units.bases and flux_or_sb_selected == 'Surface Brightness':
             spec_units /= u.sr
             # update display units
             self.spectrum_viewer.state.y_display_unit = str(spec_units)
 
+        else:
+            raise ValueError("Unrecognized unit translation, \
+                             please ensure y-unit is in Surface Brightness or Flux.")
+
     @observe('spectral_unit_selected')
     def _on_spectral_unit_changed(self, *args):
         self.hub.broadcast(GlobalDisplayUnitChanged('spectral',
@@ -167,23 +169,32 @@ def _on_flux_unit_changed(self, *args):
             self.hub.broadcast(GlobalDisplayUnitChanged('flux',
                                                         self.flux_unit.selected,
                                                         sender=self))
-    @observe('flux_or_sb_selected')
+
+    # Ensure first dropdown selection for Flux/Surface Brightness
+    # is in accordance with the data collection item's units.
+    @observe('show_translator')
     def _set_flux_or_sb(self, *args):
-        if u.sr in u.Unit(self.flux_unit.selected).bases:
-            self.flux_or_sb_selected = 'Surface Brightness'
-        else:
-            self.flux_or_sb_selected = 'Flux'        
+        if (self.spectrum_viewer and hasattr(self.spectrum_viewer.state, 'y_display_unit')
+           and self.spectrum_viewer.state.y_display_unit is not None):
+            if u.sr in u.Unit(self.spectrum_viewer.state.y_display_unit).bases:
+                self.flux_or_sb_selected = 'Surface Brightness'
+            else:
+                self.flux_or_sb_selected = 'Flux'
 
-    @observe('translate_unit', 'flux_or_sb_selected')
+    @observe('flux_or_sb_selected')
     def _translate(self, *args):
-        # make sure we have a scale factor
+        # Check for a scale factor/data passed through spectral extraction plugin.
         specs_w_factor = [spec for spec in self.app.data_collection
                           if "_pixel_scale_factor" in spec.meta]
-
+        # Translate if we have a scale factor
         if specs_w_factor:
             self.translate_units(self.flux_or_sb_selected)
-        if not self.show_translator:
+        # The translator dropdown hasn't been loaded yet so don't try translating
+        elif not self.show_translator:
             return
+        # Notify the user to extract a spectrum before using the surface brightness/flux
+        # translation. Can be removed after all 1D spectra in Cubeviz pass through
+        # spectral extraction plugin (as the scale factor will then be stored).
         else:
             raise ValueError("No collapsed spectra in data collection, \
                               please collapse a spectrum first.")

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

@@ -26,7 +26,7 @@
         :items="flux_or_sb_items.map(i => i.label)"
         v-model="flux_or_sb_selected"
         label="Flux or Surface Brightness"
-        hint="Select between Flux and Surface Brightness global display unit for y-axis."
+        hint="Select between Flux or Surface Brightness physical type for y-axis."
         persistent-hint
       ></v-select>
     </v-row>
@@ -40,7 +40,6 @@
         :label="flux_or_sb_selected === 'Flux' ? 'Flux Unit' : 'Surface Brightness Unit'"
         :hint="flux_or_sb_selected === 'Flux' ? 'Global display unit for flux.' : 'Global display unit for surface brightness.'"
         persistent-hint
-        :disabled="config === 'cubeviz'"
       ></v-select>
     </v-row>
   </j-tray-plugin>

jdaviz/core/validunits.py

@@ -69,23 +69,23 @@ def create_flux_equivalencies_list(flux_unit, spectral_axis_unit):
     # Get local units.
     if u.sr not in flux_unit.bases:
         locally_defined_flux_units = ['Jy', 'mJy', 'uJy', 'MJy', 'Jy',
-                                    'W / (m2 Hz)',
-                                    'eV / (s m2 Hz)',
-                                    'erg / (s cm2)',
-                                    'erg / (s cm2 Angstrom)',
-                                    'erg / (s cm2 Hz)',
-                                    'ph / (s cm2 Angstrom)',
-                                    'ph / (s cm2 Hz)']
+                                      'W / (m2 Hz)',
+                                      'eV / (s m2 Hz)',
+                                      'erg / (s cm2)',
+                                      'erg / (s cm2 Angstrom)',
+                                      'erg / (s cm2 Hz)',
+                                      'ph / (s cm2 Angstrom)',
+                                      'ph / (s cm2 Hz)']
         local_units = [u.Unit(unit) for unit in locally_defined_flux_units]
     else:
         locally_defined_flux_units = ['Jy / sr', 'mJy / sr', 'uJy / sr', 'MJy / sr', 'Jy / sr',
-                                    'W / (m2 Hz sr)',
-                                    'eV / (s m2 Hz sr)',
-                                    'erg / (s cm2 sr)',
-                                    'erg / (s cm2 Angstrom sr)',
-                                    'erg / (s cm2 Hz sr)',
-                                    'ph / (s cm2 Angstrom sr)',
-                                    'ph / (s cm2 Hz sr)']
+                                      'W / (m2 Hz sr)',
+                                      'eV / (s m2 Hz sr)',
+                                      'erg / (s cm2 sr)',
+                                      'erg / (s cm2 Angstrom sr)',
+                                      'erg / (s cm2 Hz sr)',
+                                      'ph / (s cm2 Angstrom sr)',
+                                      'ph / (s cm2 Hz sr)']
         local_units = [u.Unit(unit) for unit in locally_defined_flux_units]
 
     # Remove overlap units.