From 8f99b93e36030b7ce662b912ca72f482af1b9698 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 5 Jul 2024 12:28:00 -0400 Subject: [PATCH 01/29] UI functioning unit conversion, need to reconcile tests --- jdaviz/app.py | 12 +- .../spectral_extraction.py | 10 ++ .../tests/test_unit_conversion.py | 38 +---- .../unit_conversion/unit_conversion.py | 150 +++++++++++++----- .../unit_conversion/unit_conversion.vue | 19 ++- jdaviz/core/validunits.py | 83 +++++++--- 6 files changed, 211 insertions(+), 101 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index a03b197b6e..f30f1f1392 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -79,17 +79,17 @@ def equivalent_units(self, data, cid, units): 'erg / (s cm2)', 'erg / (s cm2 Angstrom)', 'erg / (s cm2 Angstrom)', 'erg / (s cm2 Hz)', 'erg / (Hz s cm2)', - 'ph / (s cm2 Angstrom)', 'ph / (s cm2 Angstrom)', + 'ph / (Angstrom s cm2)', 'ph / (Hz s cm2)', 'ph / (Hz s cm2)', 'bol', 'AB', 'ST' ] + [ 'Jy / sr', 'mJy / sr', 'uJy / sr', 'MJy / sr', 'W / (Hz sr m2)', - '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)', - 'bol / sr', 'AB / sr', 'ST / sr' + 'eV / (Hz s sr m2)', + 'erg / (s sr cm2)', + #'erg / (s cm2 Angstrom sr)', 'erg / (Hz s sr cm2)', + #'ph / (Angstrom s sr cm2)', 'ph / (Hz s sr cm2)', + 'AB / sr' #, 'ST / sr', 'bol / sr' ]) else: # spectral axis # prefer Hz over Bq and um over micron diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py index 89d0aa16dc..6b130269a1 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py @@ -496,6 +496,16 @@ def extract(self, return_bg=False, add_data=True, **kwargs): # per https://jwst-docs.stsci.edu/jwst-near-infrared-camera/nircam-performance/nircam-absolute-flux-calibration-and-zeropoints # noqa pix_scale_factor = self.aperture_area_along_spectral * self.spectral_cube.meta.get('PIXAR_SR', 1.0) # noqa spec.meta['_pixel_scale_factor'] = pix_scale_factor + + # need this to work + ''' + if hasattr(spec.meta, '_pixel_scale_factor') and pix_scale_factor == 1: + snackbar_message = SnackbarMessage( + "PIXAR_SR FITS header keyword not found when parsing spectral cube. Flux/Surface Brightness conversion will not work.", + color="error", + sender=self) + self.hub.broadcast(snackbar_message) + ''' # stuff for exporting to file self.extracted_spec = spec diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py index 4d00c26409..59648060f8 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py @@ -140,23 +140,9 @@ def test_unit_translation(cubeviz_helper): cubeviz_helper.load_regions(CirclePixelRegion(center, radius=2.5)) uc_plg = cubeviz_helper.plugins['Unit Conversion'] - # we can get rid of this after all spectra pass through - # spectral extraction plugin - extract_plg = cubeviz_helper.plugins['Spectral Extraction'] - - extract_plg.aperture = extract_plg.aperture.choices[-1] - extract_plg.aperture_method.selected = 'Exact' - extract_plg.wavelength_dependent = True - extract_plg.function = 'Sum' - # set so pixel scale factor != 1 - extract_plg.reference_spectral_value = 0.000001 - - # all spectra will pass through spectral extraction, - # this will store a scale factor for use in translations. - collapsed_spec = extract_plg.extract() # test that the scale factor was set - assert np.all(collapsed_spec.meta['_pixel_scale_factor'] != 1) + assert np.all(cubeviz_helper.app.data_collection[-1].meta['_pixel_scale_factor'] != 1) # When the dropdown is displayed, this ensures the loaded # data collection item units will be used for translations. @@ -167,13 +153,9 @@ def test_unit_translation(cubeviz_helper): viewer_1d = cubeviz_helper.app.get_viewer( cubeviz_helper._default_spectrum_viewer_reference_name) - # for testing _set_flux_or_sb() - uc_plg._obj.show_translator = False - # change global y-units from Flux -> Surface Brightness uc_plg._obj.flux_or_sb_selected = 'Surface Brightness' - uc_plg._obj.show_translator = True assert uc_plg._obj.flux_or_sb_selected == 'Surface Brightness' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) @@ -204,34 +186,24 @@ def test_sb_unit_conversion(cubeviz_helper): uc_plg.flux_or_sb.selected = 'Surface Brightness' # Surface Brightness conversion - uc_plg.flux_or_sb_unit = 'Jy / sr' + uc_plg.sb_unit = 'Jy / sr' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) assert y_display_unit == u.Jy / u.sr # Try a second conversion - uc_plg.flux_or_sb_unit = 'W / Hz sr m2' + uc_plg.sb_unit = 'W / Hz sr m2' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) assert y_display_unit == u.Unit("W / (Hz sr m2)") # really a translation test, test_unit_translation loads a Flux # cube, this test load a Surface Brightness Cube, this ensures # two-way translation - uc_plg.flux_or_sb_unit = 'MJy / sr' + uc_plg.sb_unit = 'MJy / sr' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) - # we can get rid of this after all spectra pass through - # spectral extraction plugin - extract_plg = cubeviz_helper.plugins['Spectral Extraction'] - extract_plg.aperture = extract_plg.aperture.choices[-1] - extract_plg.aperture_method.selected = 'Exact' - extract_plg.wavelength_dependent = True - extract_plg.function = 'Sum' - extract_plg.reference_spectral_value = 0.000001 - extract_plg.extract() - uc_plg._obj.show_translator = True uc_plg._obj.flux_or_sb_selected = 'Flux' - uc_plg.flux_or_sb_unit = 'MJy' + uc_plg.flux_unit = 'MJy' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) assert y_display_unit == u.MJy diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 18266de424..37d65aa291 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -7,7 +7,9 @@ from jdaviz.core.template_mixin import (PluginTemplateMixin, UnitSelectPluginComponent, SelectPluginComponent, PluginUserApi) from jdaviz.core.validunits import (create_spectral_equivalencies_list, - create_flux_equivalencies_list) + create_flux_equivalencies_list, + create_sb_equivalencies_list, + check_if_unit_is_per_solid_angle) __all__ = ['UnitConversion'] @@ -57,10 +59,15 @@ class UnitConversion(PluginTemplateMixin): flux_unit_items = List().tag(sync=True) flux_unit_selected = Unicode().tag(sync=True) + sb_unit_items = List().tag(sync=True) + sb_unit_selected = Unicode().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) + can_translate = Bool(True).tag(sync=True) + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -76,23 +83,30 @@ def __init__(self, *args, **kwargs): # TODO [markers]: existing markers need converting self.spectrum_viewer.state.add_callback('x_display_unit', self._on_glue_x_display_unit_changed) + self.spectrum_viewer.state.add_callback('y_display_unit', self._on_glue_y_display_unit_changed) self.spectral_unit = UnitSelectPluginComponent(self, items='spectral_unit_items', selected='spectral_unit_selected') - self.flux_or_sb_unit = UnitSelectPluginComponent(self, - items='flux_unit_items', - selected='flux_unit_selected') + self.flux_or_sb = SelectPluginComponent(self, items='flux_or_sb_items', selected='flux_or_sb_selected', manual_options=['Surface Brightness', 'Flux']) + + self.flux_unit = UnitSelectPluginComponent(self, + items='flux_unit_items', + selected='flux_unit_selected') + + self.sb_unit = UnitSelectPluginComponent(self, + items='sb_unit_items', + selected='sb_unit_selected') @property def user_api(self): - return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_or_sb_unit')) + return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_or_sb_unit', 'flux_unit', 'sb_unit')) def _on_glue_x_display_unit_changed(self, x_unit): if x_unit is None: @@ -110,7 +124,7 @@ def _on_glue_x_display_unit_changed(self, x_unit): # which would then be appended on to the list of choices going forward self.spectral_unit._addl_unit_strings = self.spectrum_viewer.state.__class__.x_display_unit.get_choices(self.spectrum_viewer.state) # noqa self.spectral_unit.selected = x_unit - if not len(self.flux_or_sb_unit.choices): + if not len(self.flux_unit.choices) or not len(self.sb_unit.choices): # in case flux_unit was triggered first (but could not be set because there # as no spectral_unit to determine valid equivalencies) self._on_glue_y_display_unit_changed(self.spectrum_viewer.state.y_display_unit) @@ -124,31 +138,59 @@ def _on_glue_y_display_unit_changed(self, y_unit): # and call this manually in the case that that is triggered second. return self.spectrum_viewer.set_plot_axes() - if y_unit != self.flux_or_sb_unit.selected: + + flux_or_sb = '' + if check_if_unit_is_per_solid_angle(y_unit): + flux_or_sb = 'Surface Brightness' + else: + flux_or_sb = 'Flux' + + if flux_or_sb == 'Flux' and y_unit != self.flux_unit.selected: x_u = u.Unit(self.spectral_unit.selected) y_unit = _valid_glue_display_unit(y_unit, self.spectrum_viewer, 'y') y_u = u.Unit(y_unit) - choices = create_flux_equivalencies_list(y_u, x_u) + flux_choices = create_flux_equivalencies_list(y_u, x_u) + sb_choices = create_sb_equivalencies_list(y_u * u.sr, x_u) # ensure that original entry is in the list of choices - if not np.any([y_u == u.Unit(choice) for choice in choices]): - choices = [y_unit] + choices - self.flux_or_sb_unit.choices = choices - self.flux_or_sb_unit.selected = y_unit + if not np.any([y_u == u.Unit(choice) for choice in flux_choices]): + flux_choices = [y_unit] + flux_choices + self.flux_unit.choices = flux_choices + self.sb_unit.choices = sb_choices + if not self.sb_unit.selected: + self.sb_unit.selected = str((y_u) / u.sr) + self.flux_unit.selected = y_unit + + elif flux_or_sb == 'Surface Brightness' and y_unit != self.sb_unit.selected: + x_u = u.Unit(self.spectral_unit.selected) + y_unit = _valid_glue_display_unit(y_unit, self.spectrum_viewer, 'y') + y_u = u.Unit(y_unit) + flux_choices = create_flux_equivalencies_list(y_u / u.sr, x_u) + sb_choices = create_sb_equivalencies_list(y_u, x_u) + # ensure that original entry is in the list of choices + if not np.any([y_u == u.Unit(choice) for choice in sb_choices]): + sb_choices = [y_unit] + sb_choices + self.flux_unit.choices = flux_choices + self.sb_unit.choices = sb_choices + self.sb_unit.selected = y_unit + # self.flux_unit.selected = str(y_u * u.sr) + def translate_units(self, flux_or_sb_selected): - spec_units = u.Unit(self.spectrum_viewer.state.y_display_unit) + spec_units = (self.spectrum_viewer.state.y_display_unit) # Surface Brightness -> Flux - if u.sr in spec_units.bases and flux_or_sb_selected == 'Flux': + if check_if_unit_is_per_solid_angle(spec_units) 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 == 'Surface Brightness': + elif not check_if_unit_is_per_solid_angle(spec_units) and flux_or_sb_selected == 'Surface Brightness': spec_units /= u.sr # update display units self.spectrum_viewer.state.y_display_unit = str(spec_units) + + self.spectrum_viewer.reset_limits() @observe('spectral_unit_selected') def _on_spectral_unit_changed(self, *args): @@ -159,17 +201,42 @@ def _on_spectral_unit_changed(self, *args): self.spectral_unit.selected, sender=self)) - @observe('flux_unit_selected') + @observe('flux_unit_selected', 'sb_unit_selected') def _on_flux_unit_changed(self, *args): + flux_or_sb = None - yunit = _valid_glue_display_unit(self.flux_or_sb_unit.selected, self.spectrum_viewer, 'y') + for arg in args: + if arg['name'] == 'flux_unit_selected': + flux_or_sb = self.flux_unit.selected + + untranslatable = [ + 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 + ] - if self.spectrum_viewer.state.y_display_unit != yunit: + if flux_or_sb in untranslatable: + self.can_translate = False + else: + self.can_translate = True + + elif arg['name'] == 'sb_unit_selected': + flux_or_sb = self.sb_unit.selected + + self.can_translate = True + yunit = _valid_glue_display_unit(flux_or_sb, self.spectrum_viewer, 'y') + + if self.spectrum_viewer.state.y_display_unit != yunit: self.spectrum_viewer.state.y_display_unit = yunit self.hub.broadcast(GlobalDisplayUnitChanged('flux', - self.flux_or_sb_unit.selected, + flux_or_sb, sender=self)) + self.spectrum_viewer.reset_limits() # Ensure first dropdown selection for Flux/Surface Brightness # is in accordance with the data collection item's units. @@ -177,29 +244,36 @@ def _on_flux_unit_changed(self, *args): def _set_flux_or_sb(self, *args): 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' + self.flux_or_sb_selected = 'Flux' - @observe('flux_or_sb_selected') + @observe('flux_or_sb_selected', 'flux_unit_selected', 'sb_unit_selected') def _translate(self, *args): # currently unsupported, can be supported with a scale factor if self.app.config == 'specviz': return - - # 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) # The translator dropdown hasn't been loaded yet so don't try translating - elif not self.show_translator: + if 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.") + + flux_or_sb_select = None + + if args: + # need to check current y_unit to see if we need to translate + y_unit = self.spectrum_viewer.state.y_display_unit + for arg in args: + if arg['name'] == 'flux_unit_selected': + # don't translate if self.flux_or_sb == Flux + if not check_if_unit_is_per_solid_angle(y_unit): + return + flux_or_sb_select = 'Flux' + elif arg['name'] == 'sb_unit_selected': + # don't translate if self.flux_or_sb == Surface Brightness + if check_if_unit_is_per_solid_angle(y_unit): + return + flux_or_sb_select = 'Surface Brightness' + elif arg['name'] == 'flux_or_sb_selected': + flux_or_sb_select = self.flux_or_sb_selected + else: + return + + self.translate_units(flux_or_sb_select) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index ee252b246b..ea2c8050fa 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -28,7 +28,9 @@ label="Flux or Surface Brightness" hint="Select between Flux or Surface Brightness physical type for y-axis." persistent-hint + :disabled="!can_translate" > + Translation is not available due to current settings. @@ -37,10 +39,23 @@ attach :items="flux_unit_items.map(i => i.label)" v-model="flux_unit_selected" - :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.'" + label="Flux Unit" + hint="Global display unit for y-axis axis." persistent-hint > + + + + + diff --git a/jdaviz/core/validunits.py b/jdaviz/core/validunits.py index 22c440a5e6..ed0769f5c3 100644 --- a/jdaviz/core/validunits.py +++ b/jdaviz/core/validunits.py @@ -67,28 +67,17 @@ def create_flux_equivalencies_list(flux_unit, spectral_axis_unit): except u.core.UnitConversionError: return [] - # Get local units. - if u.sr not in flux_unit.bases: - locally_defined_flux_units = ['Jy', 'mJy', 'uJy', 'MJy', 'Jy', - 'W / (Hz m2)', - '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 / (Hz sr m2)', - '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)', - 'bol / sr', 'AB / sr', 'ST / sr'] - local_units = [u.Unit(unit) for unit in locally_defined_flux_units] + # Get local flux units. + locally_defined_flux_units = ['Jy', 'mJy', 'uJy', 'MJy', + 'W / (Hz m2)', + 'eV / (s m2 Hz)', + 'erg / (s cm2)', + 'erg / (s cm2 Hz)', + 'ph / (Angstrom s cm2)', + 'ph / (Hz s cm2)', + 'bol', 'AB', 'ST' + ] + local_units = [u.Unit(unit) for unit in locally_defined_flux_units] # Remove overlap units. curr_flux_unit_equivalencies = list(set(curr_flux_unit_equivalencies) @@ -100,6 +89,56 @@ def create_flux_equivalencies_list(flux_unit, spectral_axis_unit): # Concatenate both lists with the local units coming first. return sorted(units_to_strings(local_units)) + flux_unit_equivalencies_titles +def create_sb_equivalencies_list(sb_unit, spectral_axis_unit): + """Get all possible conversions for flux from current flux units.""" + if ((sb_unit in (u.count, u.dimensionless_unscaled)) + or (spectral_axis_unit in (u.pix, u.dimensionless_unscaled))): + return [] + + # Get unit equivalencies. Value passed into u.spectral_density() is irrelevant. + try: + curr_sb_unit_equivalencies = sb_unit.find_equivalent_units( + equivalencies=u.spectral_density(1 * spectral_axis_unit), + include_prefix_units=False) + except u.core.UnitConversionError: + return [] + + locally_defined_sb_units = ['Jy / sr', 'mJy / sr', 'uJy / sr', 'MJy / sr', 'Jy / sr', + 'W / (Hz sr m2)', + 'eV / (Hz s sr m2)', + #'erg / (s cm2 sr)', + #'erg / (s cm2 Angstrom sr)', + #'erg / (s cm2 Hz sr)', + #'ph / (Angstrom s sr cm2)', + #'ph / (s cm2 Hz sr)', + 'AB / sr'] + + local_units = [u.Unit(unit) for unit in locally_defined_sb_units] + + exclude = [] + jansky_units = [u.Jy, u.mJy, u.uJy, u.MJy] + + for unit in jansky_units: + if any(base in unit.bases for base in sb_unit.bases): + exclude = [ + u.ph / (u.Angstrom * u.s * u.sr * u.cm**2), + u.ph / (u.Hz * u.s * u.sr * u.cm**2), + u.ST / u.sr, u.bol / u.sr, + u.erg / (u.Angstrom * u.s * u.sr * u.cm**2), + u.erg / (u.Hz * u.s * u.sr * u.cm**2), + u.erg / (u.s * u.sr * u.cm**2) + ] + + # Remove overlap units. + curr_sb_unit_equivalencies = list(set(curr_sb_unit_equivalencies) + - set(local_units)) + + # Convert equivalencies into readable versions of the units and sort them alphabetically. + sb_unit_equivalencies_titles = sorted(units_to_strings(curr_sb_unit_equivalencies)) + + # Concatenate both lists with the local units coming first. + return sorted(units_to_strings(local_units)) + sb_unit_equivalencies_titles + def check_if_unit_is_per_solid_angle(unit): """ From a1e0b11177bed7043a67a847c7f22817ea173c22 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 5 Jul 2024 15:30:30 -0400 Subject: [PATCH 02/29] update syntax, remove unnecessary logic --- .../unit_conversion/unit_conversion.py | 41 +++++++++++-------- jdaviz/core/validunits.py | 14 ------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 37d65aa291..615d38b979 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -106,7 +106,7 @@ def __init__(self, *args, **kwargs): @property def user_api(self): - return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_or_sb_unit', 'flux_unit', 'sb_unit')) + return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_unit', 'sb_unit')) def _on_glue_x_display_unit_changed(self, x_unit): if x_unit is None: @@ -174,21 +174,20 @@ def _on_glue_y_display_unit_changed(self, y_unit): self.flux_unit.choices = flux_choices self.sb_unit.choices = sb_choices self.sb_unit.selected = y_unit - # self.flux_unit.selected = str(y_u * u.sr) def translate_units(self, flux_or_sb_selected): - spec_units = (self.spectrum_viewer.state.y_display_unit) + spec_units = self.spectrum_viewer.state.y_display_unit # Surface Brightness -> Flux if check_if_unit_is_per_solid_angle(spec_units) and flux_or_sb_selected == 'Flux': spec_units *= u.sr # update display units - self.spectrum_viewer.state.y_display_unit = str(spec_units) + self.spectrum_viewer.state.y_display_unit = spec_units # Flux -> Surface Brightness elif not check_if_unit_is_per_solid_angle(spec_units) and flux_or_sb_selected == 'Surface Brightness': spec_units /= u.sr # update display units - self.spectrum_viewer.state.y_display_unit = str(spec_units) + self.spectrum_viewer.state.y_display_unit = spec_units self.spectrum_viewer.reset_limits() @@ -204,30 +203,38 @@ def _on_spectral_unit_changed(self, *args): @observe('flux_unit_selected', 'sb_unit_selected') def _on_flux_unit_changed(self, *args): flux_or_sb = None + current_y = self.spectrum_viewer.state.y_display_unit for arg in args: + # determine if flux or surface brightness unit was changed by user if arg['name'] == 'flux_unit_selected': flux_or_sb = self.flux_unit.selected + # update flux or surface brightness dropdown if necessary + if check_if_unit_is_per_solid_angle(current_y): + self.flux_or_sb.selected = 'Flux' - untranslatable = [ + untranslatable_units = [ 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 + u.ST, u.bol ] - - if flux_or_sb in untranslatable: + # disable translator is flux unit is untranslatable + # still can select surface brightness units, just disables + # flux -> surface brightnes of particular unit selected. + if flux_or_sb in untranslatable_units: self.can_translate = False else: - self.can_translate = True + self.can_translate = True elif arg['name'] == 'sb_unit_selected': flux_or_sb = self.sb_unit.selected - self.can_translate = True + # update flux or surface brightness dropdown if necessary + if not check_if_unit_is_per_solid_angle(current_y): + self.flux_or_sb.selected = 'Surface Brightness' yunit = _valid_glue_display_unit(flux_or_sb, self.spectrum_viewer, 'y') @@ -255,7 +262,7 @@ def _translate(self, *args): if not self.show_translator: return - flux_or_sb_select = None + flux_or_sb = None if args: # need to check current y_unit to see if we need to translate @@ -265,15 +272,15 @@ def _translate(self, *args): # don't translate if self.flux_or_sb == Flux if not check_if_unit_is_per_solid_angle(y_unit): return - flux_or_sb_select = 'Flux' + flux_or_sb = 'Flux' elif arg['name'] == 'sb_unit_selected': # don't translate if self.flux_or_sb == Surface Brightness if check_if_unit_is_per_solid_angle(y_unit): return - flux_or_sb_select = 'Surface Brightness' + flux_or_sb = 'Surface Brightness' elif arg['name'] == 'flux_or_sb_selected': - flux_or_sb_select = self.flux_or_sb_selected + flux_or_sb = self.flux_or_sb_selected else: return - self.translate_units(flux_or_sb_select) + self.translate_units(flux_or_sb) diff --git a/jdaviz/core/validunits.py b/jdaviz/core/validunits.py index ed0769f5c3..d5705979f0 100644 --- a/jdaviz/core/validunits.py +++ b/jdaviz/core/validunits.py @@ -115,20 +115,6 @@ def create_sb_equivalencies_list(sb_unit, spectral_axis_unit): local_units = [u.Unit(unit) for unit in locally_defined_sb_units] - exclude = [] - jansky_units = [u.Jy, u.mJy, u.uJy, u.MJy] - - for unit in jansky_units: - if any(base in unit.bases for base in sb_unit.bases): - exclude = [ - u.ph / (u.Angstrom * u.s * u.sr * u.cm**2), - u.ph / (u.Hz * u.s * u.sr * u.cm**2), - u.ST / u.sr, u.bol / u.sr, - u.erg / (u.Angstrom * u.s * u.sr * u.cm**2), - u.erg / (u.Hz * u.s * u.sr * u.cm**2), - u.erg / (u.s * u.sr * u.cm**2) - ] - # Remove overlap units. curr_sb_unit_equivalencies = list(set(curr_sb_unit_equivalencies) - set(local_units)) From 6e32a931cc1957dfe396b81dbdf6ad7ec2152424 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Mon, 8 Jul 2024 12:11:48 -0400 Subject: [PATCH 03/29] tests now pass, API functionality improved --- jdaviz/app.py | 7 +- .../spectral_extraction.py | 11 ++- .../unit_conversion/unit_conversion.py | 73 ++++++++++++------- .../unit_conversion/unit_conversion.vue | 4 +- jdaviz/core/validunits.py | 15 ++-- 5 files changed, 68 insertions(+), 42 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index f30f1f1392..806558e74e 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -87,9 +87,10 @@ def equivalent_units(self, data, cid, units): 'W / (Hz sr m2)', 'eV / (Hz s sr m2)', 'erg / (s sr cm2)', - #'erg / (s cm2 Angstrom sr)', 'erg / (Hz s sr cm2)', - #'ph / (Angstrom s sr cm2)', 'ph / (Hz s sr cm2)', - 'AB / sr' #, 'ST / sr', 'bol / sr' + 'AB / sr' + # 'erg / (s cm2 Angstrom sr)', 'erg / (Hz s sr cm2)', + # 'ph / (Angstrom s sr cm2)', 'ph / (Hz s sr cm2)', + # 'ST / sr', 'bol / sr' ]) else: # spectral axis # prefer Hz over Bq and um over micron diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py index 6b130269a1..0269c6dc7a 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py @@ -496,16 +496,15 @@ def extract(self, return_bg=False, add_data=True, **kwargs): # per https://jwst-docs.stsci.edu/jwst-near-infrared-camera/nircam-performance/nircam-absolute-flux-calibration-and-zeropoints # noqa pix_scale_factor = self.aperture_area_along_spectral * self.spectral_cube.meta.get('PIXAR_SR', 1.0) # noqa spec.meta['_pixel_scale_factor'] = pix_scale_factor - - # need this to work - ''' - if hasattr(spec.meta, '_pixel_scale_factor') and pix_scale_factor == 1: + + # inform the user if scale factor keyword not in metadata + if 'PIXAR_SR' not in self.spectral_cube.meta: snackbar_message = SnackbarMessage( - "PIXAR_SR FITS header keyword not found when parsing spectral cube. Flux/Surface Brightness conversion will not work.", + ("PIXAR_SR FITS header keyword not found when parsing spectral cube. " + "Flux/Surface Brightness will use default scale factor of 1."), color="error", sender=self) self.hub.broadcast(snackbar_message) - ''' # stuff for exporting to file self.extracted_spec = spec diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 615d38b979..3ae2e6cbbe 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -9,7 +9,8 @@ from jdaviz.core.validunits import (create_spectral_equivalencies_list, create_flux_equivalencies_list, create_sb_equivalencies_list, - check_if_unit_is_per_solid_angle) + check_if_unit_is_per_solid_angle, + units_to_strings) __all__ = ['UnitConversion'] @@ -45,11 +46,12 @@ class UnitConversion(PluginTemplateMixin): * :meth:`~jdaviz.core.template_mixin.PluginTemplateMixin.close_in_tray` * ``spectral_unit`` (:class:`~jdaviz.core.template_mixin.UnitSelectPluginComponent`): Global unit to use for all spectral axes. - * ``flux_or_sb_unit`` (:class:`~jdaviz.core.template_mixin.UnitSelectPluginComponent`): - Global unit to use for all flux/surface brightness (depending on flux_or_sb selection) axes. * ``flux_or_sb`` (:class:`~jdaviz.core.template_mixin.SelectPluginComponent`): - Y-axis physical type selection. Currently only accessible in Cubeviz (pixel scale factor - added in Cubeviz Spectral Extraction, and is used for this translation). + Y-axis physical type selection. Currently only accessible in Cubeviz. + * ``flux_unit`` (:class:`~jdaviz.core.template_mixin.UnitSelectPluginComponent`): + Global flux unit to use for y-axis. + * ``sb_unit`` (:class:`~jdaviz.core.template_mixin.UnitSelectPluginComponent`): + Global surface brightness unit to use for y-axis. """ template_file = __file__, "unit_conversion.vue" @@ -83,7 +85,7 @@ def __init__(self, *args, **kwargs): # TODO [markers]: existing markers need converting self.spectrum_viewer.state.add_callback('x_display_unit', self._on_glue_x_display_unit_changed) - + self.spectrum_viewer.state.add_callback('y_display_unit', self._on_glue_y_display_unit_changed) @@ -95,7 +97,7 @@ def __init__(self, *args, **kwargs): items='flux_or_sb_items', selected='flux_or_sb_selected', manual_options=['Surface Brightness', 'Flux']) - + self.flux_unit = UnitSelectPluginComponent(self, items='flux_unit_items', selected='flux_unit_selected') @@ -145,50 +147,51 @@ def _on_glue_y_display_unit_changed(self, y_unit): else: flux_or_sb = 'Flux' + x_u = u.Unit(self.spectral_unit.selected) + y_unit = _valid_glue_display_unit(y_unit, self.spectrum_viewer, 'y') + y_u = u.Unit(y_unit) + if flux_or_sb == 'Flux' and y_unit != self.flux_unit.selected: - x_u = u.Unit(self.spectral_unit.selected) - y_unit = _valid_glue_display_unit(y_unit, self.spectrum_viewer, 'y') - y_u = u.Unit(y_unit) flux_choices = create_flux_equivalencies_list(y_u, x_u) sb_choices = create_sb_equivalencies_list(y_u * u.sr, x_u) # ensure that original entry is in the list of choices if not np.any([y_u == u.Unit(choice) for choice in flux_choices]): flux_choices = [y_unit] + flux_choices + self.flux_unit.choices = flux_choices self.sb_unit.choices = sb_choices - if not self.sb_unit.selected: - self.sb_unit.selected = str((y_u) / u.sr) self.flux_unit.selected = y_unit elif flux_or_sb == 'Surface Brightness' and y_unit != self.sb_unit.selected: - x_u = u.Unit(self.spectral_unit.selected) - y_unit = _valid_glue_display_unit(y_unit, self.spectrum_viewer, 'y') - y_u = u.Unit(y_unit) flux_choices = create_flux_equivalencies_list(y_u / u.sr, x_u) sb_choices = create_sb_equivalencies_list(y_u, x_u) # ensure that original entry is in the list of choices if not np.any([y_u == u.Unit(choice) for choice in sb_choices]): sb_choices = [y_unit] + sb_choices + self.flux_unit.choices = flux_choices self.sb_unit.choices = sb_choices self.sb_unit.selected = y_unit - + def translate_units(self, flux_or_sb_selected): - spec_units = self.spectrum_viewer.state.y_display_unit + spec_units = u.Unit(self.spectrum_viewer.state.y_display_unit) # Surface Brightness -> Flux if check_if_unit_is_per_solid_angle(spec_units) and flux_or_sb_selected == 'Flux': spec_units *= u.sr # update display units - self.spectrum_viewer.state.y_display_unit = spec_units + self.spectrum_viewer.state.y_display_unit = str(spec_units) + self.flux_or_sb.selected = 'Flux' # Flux -> Surface Brightness - elif not check_if_unit_is_per_solid_angle(spec_units) and flux_or_sb_selected == 'Surface Brightness': + elif (not check_if_unit_is_per_solid_angle(spec_units) + and flux_or_sb_selected == 'Surface Brightness'): spec_units /= u.sr # update display units - self.spectrum_viewer.state.y_display_unit = spec_units - + self.spectrum_viewer.state.y_display_unit = str(spec_units) + self.flux_or_sb.selected = 'Surface Brightness' + self.spectrum_viewer.reset_limits() @observe('spectral_unit_selected') @@ -212,7 +215,7 @@ def _on_flux_unit_changed(self, *args): # update flux or surface brightness dropdown if necessary if check_if_unit_is_per_solid_angle(current_y): self.flux_or_sb.selected = 'Flux' - + untranslatable_units = [ u.erg / (u.s * u.cm**2), u.erg / (u.s * u.cm**2 * u.Angstrom), @@ -227,7 +230,7 @@ def _on_flux_unit_changed(self, *args): if flux_or_sb in untranslatable_units: self.can_translate = False else: - self.can_translate = True + self.can_translate = True elif arg['name'] == 'sb_unit_selected': flux_or_sb = self.sb_unit.selected @@ -261,7 +264,7 @@ def _translate(self, *args): # The translator dropdown hasn't been loaded yet so don't try translating if not self.show_translator: return - + flux_or_sb = None if args: @@ -276,11 +279,29 @@ def _translate(self, *args): elif arg['name'] == 'sb_unit_selected': # don't translate if self.flux_or_sb == Surface Brightness if check_if_unit_is_per_solid_angle(y_unit): - return + return flux_or_sb = 'Surface Brightness' elif arg['name'] == 'flux_or_sb_selected': flux_or_sb = self.flux_or_sb_selected else: return - + + # we want to raise an error if a user tries to translate with an + # untranslated Flux unit using the API + untranslatable_units = [ + 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 + ] + untranslatable_units = units_to_strings(untranslatable_units) + + if self.flux_unit.selected in untranslatable_units and flux_or_sb == 'Surface Brightness': + raise ValueError( + f"Selected flux unit is not translatable. Please choose a flux unit " + f"that is not in the following list: {untranslatable_units}." + ) + self.translate_units(flux_or_sb) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index ea2c8050fa..596d15c496 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -30,7 +30,7 @@ persistent-hint :disabled="!can_translate" > - Translation is not available due to current settings. + Translation is not available due to current unit selection. @@ -54,7 +54,9 @@ label="Surface Brightness Unit" hint="Global display unit for y-axis axis." persistent-hint + :disabled="!can_translate" > + Translation is not available due to current unit selection. diff --git a/jdaviz/core/validunits.py b/jdaviz/core/validunits.py index d5705979f0..0ff6e9671a 100644 --- a/jdaviz/core/validunits.py +++ b/jdaviz/core/validunits.py @@ -73,6 +73,7 @@ def create_flux_equivalencies_list(flux_unit, spectral_axis_unit): 'eV / (s m2 Hz)', 'erg / (s cm2)', 'erg / (s cm2 Hz)', + 'erg / (s cm2 Angstrom)', 'ph / (Angstrom s cm2)', 'ph / (Hz s cm2)', 'bol', 'AB', 'ST' @@ -89,6 +90,7 @@ def create_flux_equivalencies_list(flux_unit, spectral_axis_unit): # Concatenate both lists with the local units coming first. return sorted(units_to_strings(local_units)) + flux_unit_equivalencies_titles + def create_sb_equivalencies_list(sb_unit, spectral_axis_unit): """Get all possible conversions for flux from current flux units.""" if ((sb_unit in (u.count, u.dimensionless_unscaled)) @@ -106,12 +108,13 @@ def create_sb_equivalencies_list(sb_unit, spectral_axis_unit): locally_defined_sb_units = ['Jy / sr', 'mJy / sr', 'uJy / sr', 'MJy / sr', 'Jy / sr', 'W / (Hz sr m2)', 'eV / (Hz s sr m2)', - #'erg / (s cm2 sr)', - #'erg / (s cm2 Angstrom sr)', - #'erg / (s cm2 Hz sr)', - #'ph / (Angstrom s sr cm2)', - #'ph / (s cm2 Hz sr)', - 'AB / sr'] + 'AB / sr' + # 'erg / (s cm2 sr)', + # 'erg / (s cm2 Angstrom sr)', + # 'erg / (s cm2 Hz sr)', + # 'ph / (Angstrom s sr cm2)', + # 'ph / (s cm2 Hz sr)' + ] local_units = [u.Unit(unit) for unit in locally_defined_sb_units] From 3013fb8a6b5bb18c018bb70c672de3726d1c2dd6 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Mon, 8 Jul 2024 12:51:59 -0400 Subject: [PATCH 04/29] add additional equivalencies for coords info --- jdaviz/configs/imviz/plugins/coords_info/coords_info.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index 87674a5253..b1306b198d 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -562,8 +562,9 @@ def _copy_axes_to_spectral(): if '_pixel_scale_factor' in sp.meta: eqv = [(u.MJy / u.sr, u.MJy, - lambda x: (x * sp.meta['_pixel_scale_factor']), + lambda x: (x * np.array(sp.meta.get('_pixel_scale_factor', 1))), lambda x: x)] + eqv += u.spectral_density(sp.spectral_axis) disp_flux = sp.flux.to_value(viewer.state.y_display_unit, eqv) else: disp_flux = sp.flux.to_value(viewer.state.y_display_unit, From 69cce01c2e8280e6d682dec2c3b91a3bb6111559 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 10 Jul 2024 14:25:08 -0400 Subject: [PATCH 05/29] first pass at disabling specviz translations --- .../unit_conversion/unit_conversion.py | 30 ++++++++++++++++++- .../unit_conversion/unit_conversion.vue | 4 +-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 3ae2e6cbbe..1cf169d409 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -68,6 +68,7 @@ class UnitConversion(PluginTemplateMixin): flux_or_sb_items = List().tag(sync=True) flux_or_sb_selected = Unicode().tag(sync=True) + specviz_disabler = Unicode().tag(sync=True) can_translate = Bool(True).tag(sync=True) def __init__(self, *args, **kwargs): @@ -108,7 +109,15 @@ def __init__(self, *args, **kwargs): @property def user_api(self): - return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_unit', 'sb_unit')) + if self.app.config == 'cubeviz': + return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_unit', 'sb_unit')) + elif self.app.config == 'specviz' and not self.specviz_disabler: + return PluginUserApi(self, expose=('spectral_unit', 'flux_unit', 'sb_unit')) + if self.specviz_disabler == 'Flux': + return PluginUserApi(self, expose=('spectral_unit', 'sb_unit')) + if self.specviz_disabler == 'Surface Brightness': + return PluginUserApi(self, expose=('spectral_unit', 'flux_unit')) + def _on_glue_x_display_unit_changed(self, x_unit): if x_unit is None: @@ -208,9 +217,24 @@ def _on_flux_unit_changed(self, *args): flux_or_sb = None current_y = self.spectrum_viewer.state.y_display_unit + data_collection_unit = '' + if self.app.data_collection[0] and self.app.config == 'specviz': + if check_if_unit_is_per_solid_angle(self.app.data_collection[0].get_object()._unit): + data_collection_unit = 'Surface Brightness' + self.specviz_disabler = 'Flux' + self.user_api() + else: + self.specviz_disabler = 'Surface Brightness' + data_collection_unit = 'Flux' + self.user_api() + for arg in args: # determine if flux or surface brightness unit was changed by user if arg['name'] == 'flux_unit_selected': + if data_collection_unit == 'Surface Brightness': + raise ValueError( + f"Unit translation between Flux and Surface Brightness is not supported in Specviz." + ) flux_or_sb = self.flux_unit.selected # update flux or surface brightness dropdown if necessary if check_if_unit_is_per_solid_angle(current_y): @@ -233,6 +257,10 @@ def _on_flux_unit_changed(self, *args): self.can_translate = True elif arg['name'] == 'sb_unit_selected': + if data_collection_unit == 'Flux': + raise ValueError( + f"Unit translation between Flux and Surface Brightness is not supported in Specviz." + ) flux_or_sb = self.sb_unit.selected self.can_translate = True # update flux or surface brightness dropdown if necessary diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index 596d15c496..cb481be35f 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -33,7 +33,7 @@ Translation is not available due to current unit selection. - + - + Date: Thu, 11 Jul 2024 14:40:59 -0400 Subject: [PATCH 06/29] Remove dev flag, consolidate untranslatable units, multi-config support, styling changes --- .../moment_maps/tests/test_moment_maps.py | 2 +- .../unit_conversion/unit_conversion.py | 81 +++++++++---------- .../unit_conversion/unit_conversion.vue | 36 +++++---- 3 files changed, 60 insertions(+), 59 deletions(-) diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py index 545176e61e..9ba7efeffb 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py @@ -334,7 +334,7 @@ def test_correct_output_flux_or_sb_units(cubeviz_helper, spectrum1d_cube_custom_ # now change surface brightness units in the unit conversion plugin - uc.flux_or_sb_unit = 'Jy / sr' + uc.sb_unit = 'Jy / sr' # and make sure this change is propogated output_unit_moment_0 = mm.output_unit_items[0] diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 1cf169d409..2ce0150742 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -64,7 +64,6 @@ class UnitConversion(PluginTemplateMixin): sb_unit_items = List().tag(sync=True) sb_unit_selected = Unicode().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) @@ -110,14 +109,13 @@ def __init__(self, *args, **kwargs): @property def user_api(self): if self.app.config == 'cubeviz': - return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_unit', 'sb_unit')) + return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_unit', 'sb_unit')) # noqa elif self.app.config == 'specviz' and not self.specviz_disabler: return PluginUserApi(self, expose=('spectral_unit', 'flux_unit', 'sb_unit')) if self.specviz_disabler == 'Flux': return PluginUserApi(self, expose=('spectral_unit', 'sb_unit')) if self.specviz_disabler == 'Surface Brightness': return PluginUserApi(self, expose=('spectral_unit', 'flux_unit')) - def _on_glue_x_display_unit_changed(self, x_unit): if x_unit is None: @@ -162,30 +160,39 @@ def _on_glue_y_display_unit_changed(self, y_unit): if flux_or_sb == 'Flux' and y_unit != self.flux_unit.selected: flux_choices = create_flux_equivalencies_list(y_u, x_u) - sb_choices = create_sb_equivalencies_list(y_u * u.sr, x_u) # ensure that original entry is in the list of choices if not np.any([y_u == u.Unit(choice) for choice in flux_choices]): flux_choices = [y_unit] + flux_choices + if self.app.config == 'cubeviz': + sb_choices = create_sb_equivalencies_list(y_u / u.sr, x_u) + self.sb_unit.choices = sb_choices + if not self.sb_unit.selected: + self.sb_unit.selected = y_unit + " / sr" + self.flux_unit.choices = flux_choices - self.sb_unit.choices = sb_choices self.flux_unit.selected = y_unit elif flux_or_sb == 'Surface Brightness' and y_unit != self.sb_unit.selected: - flux_choices = create_flux_equivalencies_list(y_u / u.sr, x_u) sb_choices = create_sb_equivalencies_list(y_u, x_u) # ensure that original entry is in the list of choices if not np.any([y_u == u.Unit(choice) for choice in sb_choices]): sb_choices = [y_unit] + sb_choices - self.flux_unit.choices = flux_choices + if self.app.config == 'cubeviz': + flux_choices = create_flux_equivalencies_list(y_u * u.sr, x_u) + self.flux_unit.choices = flux_choices + self.sb_unit.choices = sb_choices self.sb_unit.selected = y_unit def translate_units(self, flux_or_sb_selected): - spec_units = u.Unit(self.spectrum_viewer.state.y_display_unit) + if self.spectrum_viewer.state.y_display_unit: + spec_units = u.Unit(self.spectrum_viewer.state.y_display_unit) + else: + return # Surface Brightness -> Flux if check_if_unit_is_per_solid_angle(spec_units) and flux_or_sb_selected == 'Flux': spec_units *= u.sr @@ -218,36 +225,30 @@ def _on_flux_unit_changed(self, *args): current_y = self.spectrum_viewer.state.y_display_unit data_collection_unit = '' + # need to determine the input spectrum units to disable the additional + # drop down and possiblity of translations if self.app.data_collection[0] and self.app.config == 'specviz': if check_if_unit_is_per_solid_angle(self.app.data_collection[0].get_object()._unit): data_collection_unit = 'Surface Brightness' self.specviz_disabler = 'Flux' - self.user_api() else: self.specviz_disabler = 'Surface Brightness' data_collection_unit = 'Flux' - self.user_api() for arg in args: # determine if flux or surface brightness unit was changed by user if arg['name'] == 'flux_unit_selected': if data_collection_unit == 'Surface Brightness': raise ValueError( - f"Unit translation between Flux and Surface Brightness is not supported in Specviz." - ) + "Unit translation between Flux and Surface Brightness " + "is not supported in Specviz." + ) flux_or_sb = self.flux_unit.selected # update flux or surface brightness dropdown if necessary if check_if_unit_is_per_solid_angle(current_y): self.flux_or_sb.selected = 'Flux' - untranslatable_units = [ - 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 - ] + untranslatable_units = self.untranslatable_units() # disable translator is flux unit is untranslatable # still can select surface brightness units, just disables # flux -> surface brightnes of particular unit selected. @@ -259,8 +260,9 @@ def _on_flux_unit_changed(self, *args): elif arg['name'] == 'sb_unit_selected': if data_collection_unit == 'Flux': raise ValueError( - f"Unit translation between Flux and Surface Brightness is not supported in Specviz." - ) + "Unit translation between Flux and Surface Brightness " + "is not supported in Specviz." + ) flux_or_sb = self.sb_unit.selected self.can_translate = True # update flux or surface brightness dropdown if necessary @@ -276,22 +278,11 @@ def _on_flux_unit_changed(self, *args): sender=self)) self.spectrum_viewer.reset_limits() - # 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 (self.spectrum_viewer and hasattr(self.spectrum_viewer.state, 'y_display_unit') - and self.spectrum_viewer.state.y_display_unit is not None): - self.flux_or_sb_selected = 'Flux' - @observe('flux_or_sb_selected', 'flux_unit_selected', 'sb_unit_selected') def _translate(self, *args): # currently unsupported, can be supported with a scale factor if self.app.config == 'specviz': return - # The translator dropdown hasn't been loaded yet so don't try translating - if not self.show_translator: - return flux_or_sb = None @@ -316,6 +307,20 @@ def _translate(self, *args): # we want to raise an error if a user tries to translate with an # untranslated Flux unit using the API + untranslatable_units = self.untranslatable_units() + untranslatable_units = units_to_strings(untranslatable_units) + + if hasattr(self, 'flux_unit'): + if (self.flux_unit.selected in untranslatable_units) \ + and (flux_or_sb == 'Surface Brightness'): + raise ValueError( + f"Selected flux unit is not translatable. Please choose a flux unit " + f"that is not in the following list: {untranslatable_units}." + ) + + self.translate_units(flux_or_sb) + + def untranslatable_units(self): untranslatable_units = [ u.erg / (u.s * u.cm**2), u.erg / (u.s * u.cm**2 * u.Angstrom), @@ -324,12 +329,4 @@ def _translate(self, *args): u.ph / (u.s * u.cm**2 * u.Hz), u.ST, u.bol ] - untranslatable_units = units_to_strings(untranslatable_units) - - if self.flux_unit.selected in untranslatable_units and flux_or_sb == 'Surface Brightness': - raise ValueError( - f"Selected flux unit is not translatable. Please choose a flux unit " - f"that is not in the following list: {untranslatable_units}." - ) - - self.translate_units(flux_or_sb) + return untranslatable_units diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index cb481be35f..8b28bc021e 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -19,21 +19,7 @@ > - - - Translation is not available due to current unit selection. - - - + - + Translation is not available due to current unit selection. + + + + + + + Translation is not available due to current unit selection. + + From 24ff03a11600142b399397e396723a35df151882 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Tue, 2 Jul 2024 14:36:56 -0400 Subject: [PATCH 07/29] Refactor moment map unit conversion to not require spectrum, so nothing to centralize now. Fix devdeps Simplify flux_conversion logic and other minor fixes. Add flux conversion tests. Clarify which one should be reverted in the future --- .../plugins/moment_maps/moment_maps.py | 17 +--- .../moment_maps/tests/test_moment_maps.py | 17 ++-- jdaviz/tests/test_utils.py | 64 +++++++++++++- jdaviz/utils.py | 86 +++++++++---------- 4 files changed, 113 insertions(+), 71 deletions(-) diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py index f2af63cd83..3c3fc0a1c1 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py @@ -22,7 +22,7 @@ with_spinner) from jdaviz.core.validunits import check_if_unit_is_per_solid_angle from jdaviz.core.user_api import PluginUserApi -from jdaviz.utils import flux_conversion +from jdaviz.utils import _eqv_pixar_sr __all__ = ['MomentMap'] @@ -358,18 +358,9 @@ def calculate_moment(self, add_data=True): moment_new_unit = flux_or_sb_display_unit else: moment_new_unit = flux_or_sb_display_unit * self.spectrum_viewer.state.x_display_unit # noqa: E501 - - # Create a temporary Spectrum1D object with ability to convert from surface brightness - # to flux - temp_spec = Spectrum1D(flux=self.moment) - flux_values = np.sum(np.ones_like(temp_spec.flux.value), axis=(0, 1)) - pix_scale = self.dataset.selected_dc_item.meta.get('PIXAR_SR', 1.0) - pix_scale_factor = (flux_values * pix_scale) - temp_spec.meta['_pixel_scale_factor'] = pix_scale_factor - converted_spec = flux_conversion(temp_spec, self.moment.value, - self.moment.unit, - moment_new_unit) * moment_new_unit - self.moment = converted_spec + # TODO: This can be removed when we remove SB->flux unit support from Moment Maps + self.moment = self.moment.to(moment_new_unit, _eqv_pixar_sr( + self.moment.size * self.dataset.selected_dc_item.meta.get('PIXAR_SR', 1.0))) # Reattach the WCS so we can load the result self.moment = CCDData(self.moment, wcs=data_wcs) diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py index 9ba7efeffb..9f19e64a49 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py @@ -298,8 +298,10 @@ def test_momentmap_nirspec_prism(cubeviz_helper, tmp_path): def test_correct_output_flux_or_sb_units(cubeviz_helper, spectrum1d_cube_custom_fluxunit): if SPECUTILS_LT_1_15_1: moment_unit = "Jy / sr" + moment_unit_no_sr = "Jy" else: moment_unit = "Jy m / sr" + moment_unit_no_sr = "Jy m" # test that the output unit labels in the moment map plugin reflect any # changes made in the unit conversion plugin. @@ -345,20 +347,15 @@ def test_correct_output_flux_or_sb_units(cubeviz_helper, spectrum1d_cube_custom_ mm.calculate_moment() assert mm.moment.unit == moment_unit + # TODO: All this below can be removed when we remove SB->flux unit support from Moment Maps + uc.flux_or_sb.selected = 'Flux' mm._set_data_units() - # and make sure this change is propogated + # and make sure this change is propagated output_unit_moment_0 = mm.output_unit_items[0] assert output_unit_moment_0['label'] == 'Flux' assert output_unit_moment_0['unit_str'] == 'Jy' - # TODO: Failing because of dev version of upstream dependency, figure - # out which one - # assert mm.calculate_moment() - - # TODO: This test should pass once continuum subtraction works with - # flux to surface brightness conversion - # mm.continuum.selected = 'Surrounding' - # - # assert mm.calculate_moment() + mm.calculate_moment() + assert mm.moment.unit == moment_unit_no_sr diff --git a/jdaviz/tests/test_utils.py b/jdaviz/tests/test_utils.py index de9aac78c5..bbb49300cd 100644 --- a/jdaviz/tests/test_utils.py +++ b/jdaviz/tests/test_utils.py @@ -4,17 +4,77 @@ import photutils import pytest from asdf.exceptions import AsdfWarning +from astropy import units as u from astropy.utils import minversion from astropy.wcs import FITSFixedWarning +from numpy.testing import assert_allclose +from specutils import Spectrum1D + from jdaviz import utils +from jdaviz.utils import alpha_index, download_uri_to_path, flux_conversion PHOTUTILS_LT_1_12_1 = not minversion(photutils, "1.12.1.dev") +def test_spec_sb_flux_conversion(): + # Actual spectrum content does not matter, just the meta is used here. + spec = Spectrum1D(flux=[1, 1, 1] * u.Jy, spectral_axis=[1, 2, 3] * u.um) + + # values != 2 + values = [10, 20, 30] + + # Float scalar pixel scale factor + spec.meta["_pixel_scale_factor"] = 0.1 + assert_allclose(flux_conversion(spec, values, u.Jy / u.sr, u.Jy), [1, 2, 3]) + assert_allclose(flux_conversion(spec, values, u.Jy, u.Jy / u.sr), [100, 200, 300]) + + # Quantity scalar pixel scale factor + spec.meta["_pixel_scale_factor"] = 0.1 * (u.sr / u.pix) + assert_allclose(flux_conversion(spec, values, u.Jy / u.sr, u.Jy), [1, 2, 3]) + assert_allclose(flux_conversion(spec, values, u.Jy, u.Jy / u.sr), [100, 200, 300]) + + # values == 2 + values = [10, 20] + assert_allclose(flux_conversion(spec, values, u.Jy / u.sr, u.Jy), [1, 2]) + assert_allclose(flux_conversion(spec, values, u.Jy, u.Jy / u.sr), [100, 200]) + + # float array pixel scale factor + spec.meta["_pixel_scale_factor"] = [0.1, 0.2, 0.3] # min_max = [0.1, 0.3] + assert_allclose(flux_conversion(spec, values, u.Jy / u.sr, u.Jy), [1, 6]) + assert_allclose(flux_conversion(spec, values, u.Jy, u.Jy / u.sr), [100, 66.66666666666667]) + + # Quantity array pixel scale factor + spec.meta["_pixel_scale_factor"] = [0.1, 0.2, 0.3] * (u.sr / u.pix) # min_max = [0.1, 0.3] + assert_allclose(flux_conversion(spec, values, u.Jy / u.sr, u.Jy), [1, 6]) + assert_allclose(flux_conversion(spec, values, u.Jy, u.Jy / u.sr), [100, 66.66666666666667]) + + # values != 2 + values = [10, 20, 30] + spec.meta["_pixel_scale_factor"] = [0.1, 0.2, 0.3] + assert_allclose(flux_conversion(spec, values, u.Jy / u.sr, u.Jy), [1, 4, 9]) + assert_allclose(flux_conversion(spec, values, u.Jy, u.Jy / u.sr), 100) + + # values != 2 but _pixel_scale_factor size mismatch + with pytest.raises(ValueError, match="operands could not be broadcast together"): + spec.meta["_pixel_scale_factor"] = [0.1, 0.2, 0.3, 0.4] + flux_conversion(spec, values, u.Jy / u.sr, u.Jy) + + # Other kind of flux conversion unrelated to _pixel_scale_factor. + # The answer was obtained from synphot unit conversion. + spec.meta["_pixel_scale_factor"] = 0.1 + targ = [2.99792458e-12, 1.49896229e-12, 9.99308193e-13] * (u.erg / (u.AA * u.cm * u.cm * u.s)) # FLAM # noqa: E501 + assert_allclose(flux_conversion(spec, values, u.Jy, targ.unit), targ.value) + + # values == 2 (only used spec.spectral_axis[0] for some reason) + values = [10, 20] + targ = [2.99792458e-12, 5.99584916e-12] * (u.erg / (u.AA * u.cm * u.cm * u.s)) # FLAM + assert_allclose(flux_conversion(spec, values, u.Jy, targ.unit), targ.value) + + @pytest.mark.parametrize("test_input,expected", [(0, 'a'), (1, 'b'), (25, 'z'), (26, 'aa'), (701, 'zz'), (702, '{a')]) def test_alpha_index(test_input, expected): - assert utils.alpha_index(test_input) == expected + assert alpha_index(test_input) == expected def test_alpha_index_exceptions(): @@ -54,7 +114,7 @@ def test_url_to_download_imviz_local_path_warning(imviz_helper): def test_uri_to_download_specviz_local_path_check(): uri = "mast:JWST/product/jw02732-o004_t004_miri_ch1-shortmediumlong_x1d.fits" - local_path = utils.download_uri_to_path(uri, cache=False, dryrun=True) # No download + local_path = download_uri_to_path(uri, cache=False, dryrun=True) # No download # Wrong: '.\\JWST/product/jw02732-o004_t004_miri_ch1-shortmediumlong_x1d.fits' # Correct: '.\\jw02732-o004_t004_miri_ch1-shortmediumlong_x1d.fits' diff --git a/jdaviz/utils.py b/jdaviz/utils.py index ed79ca2a79..972256e81a 100644 --- a/jdaviz/utils.py +++ b/jdaviz/utils.py @@ -3,6 +3,7 @@ import threading import warnings from collections import deque +from collections.abc import Iterable from urllib.parse import urlparse import numpy as np @@ -294,7 +295,7 @@ def flux_conversion(spec, values, original_units, target_units): Parameters ---------- - spec : ~specutils.Spectrum1D~ object + spec : `~specutils.Spectrum1D` object The Spectrum1D object that will have converted flux units. values : float array @@ -308,7 +309,8 @@ def flux_conversion(spec, values, original_units, target_units): Returns ------- - Flux values in the target units. + result : float array + Flux values in the target units. """ # If there are only two values, this is likely the limits being converted, so then # in case we need to use the spectral density equivalency, we need to provide only @@ -318,57 +320,49 @@ def flux_conversion(spec, values, original_units, target_units): else: spectral_values = spec.spectral_axis + # Need this for setting the y-limits + eqv = u.spectral_density(spectral_values) + + orig_units = u.Unit(original_units) + orig_bases = orig_units.bases + targ_units = u.Unit(target_units) + targ_bases = targ_units.bases + # Ensure a spectrum passed through Spectral Extraction plugin - if '_pixel_scale_factor' in spec.meta and len(values) != 2: + if (('_pixel_scale_factor' in spec.meta) and + (((u.sr in orig_bases) and (u.sr not in targ_bases)) or + ((u.sr not in orig_bases) and (u.sr in targ_bases)))): # Data item in data collection does not update from conversion/translation. - # App wide original data units are used for conversion, original and + # App-wide original data units are used for conversion, original and # target_units dictate the conversion to take place. + n_values = len(values) + + # Make sure they are float (can be Quantity). + fac = spec.meta['_pixel_scale_factor'] + if isinstance(fac, Quantity): + fac = fac.value - if (u.sr in u.Unit(original_units).bases) and \ - (u.sr not in u.Unit(target_units).bases): - # Surface Brightness -> Flux - eqv = [(u.MJy / u.sr, - u.MJy, - lambda x: (x * np.array(spec.meta.get('_pixel_scale_factor', 1))), - lambda x: x)] - elif (u.sr not in u.Unit(original_units).bases) and \ - (u.sr in u.Unit(target_units).bases): - # Flux -> Surface Brightness - eqv = [(u.MJy, - u.MJy / u.sr, - lambda x: (x / np.array(spec.meta.get('_pixel_scale_factor', 1))), - lambda x: x)] + # Get min and max scale factors, to use with min and max of spec for y-limits. + if n_values == 2 and isinstance(fac, Iterable): + eqv_in = [min(fac), max(fac)] else: - eqv = u.spectral_density(spectral_values) - - elif len(values) == 2: - # Need this for setting the y-limits - eqv = u.spectral_density(spectral_values) - - if '_pixel_scale_factor' in spec.meta: - # get min and max scale factors, to use with min and max of spec for - # y-limits. Make sure they are Quantities (can be numpy.float64). - pixel_scale_min = (Quantity(min(spec.meta.get('_pixel_scale_factor', 1)))).value - pixel_scale_max = (Quantity(max(spec.meta.get('_pixel_scale_factor', 1)))).value - min_max = [pixel_scale_min, pixel_scale_max] - - if (u.sr in u.Unit(original_units).bases) and \ - (u.sr not in u.Unit(target_units).bases): - eqv += [(u.MJy, - u.MJy / u.sr, - lambda x: x * np.array(min_max), - lambda x: x)] - elif (u.sr not in u.Unit(original_units).bases) and \ - (u.sr in u.Unit(target_units).bases): - eqv += [(u.MJy / u.sr, - u.MJy, - lambda x: x / np.array(min_max), - lambda x: x)] + eqv_in = fac - else: - eqv = u.spectral_density(spectral_values) + eqv += _eqv_pixar_sr(np.array(eqv_in)) - return (values * u.Unit(original_units)).to_value(u.Unit(target_units), equivalencies=eqv) + return (values * orig_units).to_value(targ_units, equivalencies=eqv) + + +def _eqv_pixar_sr(pixar_sr): + def converter_flux(x): # Surface Brightness -> Flux + return x * pixar_sr + + def iconverter_flux(x): # Flux -> Surface Brightness + return x / pixar_sr + + return [(u.MJy / u.sr, u.MJy, converter_flux, iconverter_flux), + # TODO: This can be removed when we remove SB->flux unit support from Moment Maps + (u.MJy * u.m / u.sr, u.MJy * u.m, converter_flux, iconverter_flux)] def spectral_axis_conversion(values, original_units, target_units): From 59eb9f23b0c4cf61057aef9d20e68e890204eeec Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:02:07 -0400 Subject: [PATCH 08/29] Remove SB/flux conversion for moment map because moment map always in SB --- .../cubeviz/plugins/moment_maps/moment_maps.py | 6 +----- .../plugins/moment_maps/tests/test_moment_maps.py | 15 --------------- jdaviz/utils.py | 4 +--- 3 files changed, 2 insertions(+), 23 deletions(-) diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py index 3c3fc0a1c1..b0089f99d6 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py @@ -22,8 +22,6 @@ with_spinner) from jdaviz.core.validunits import check_if_unit_is_per_solid_angle from jdaviz.core.user_api import PluginUserApi -from jdaviz.utils import _eqv_pixar_sr - __all__ = ['MomentMap'] @@ -358,9 +356,7 @@ def calculate_moment(self, add_data=True): moment_new_unit = flux_or_sb_display_unit else: moment_new_unit = flux_or_sb_display_unit * self.spectrum_viewer.state.x_display_unit # noqa: E501 - # TODO: This can be removed when we remove SB->flux unit support from Moment Maps - self.moment = self.moment.to(moment_new_unit, _eqv_pixar_sr( - self.moment.size * self.dataset.selected_dc_item.meta.get('PIXAR_SR', 1.0))) + self.moment = self.moment.to(moment_new_unit) # Reattach the WCS so we can load the result self.moment = CCDData(self.moment, wcs=data_wcs) diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py index 9f19e64a49..c2efc22c4b 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py @@ -298,10 +298,8 @@ def test_momentmap_nirspec_prism(cubeviz_helper, tmp_path): def test_correct_output_flux_or_sb_units(cubeviz_helper, spectrum1d_cube_custom_fluxunit): if SPECUTILS_LT_1_15_1: moment_unit = "Jy / sr" - moment_unit_no_sr = "Jy" else: moment_unit = "Jy m / sr" - moment_unit_no_sr = "Jy m" # test that the output unit labels in the moment map plugin reflect any # changes made in the unit conversion plugin. @@ -346,16 +344,3 @@ def test_correct_output_flux_or_sb_units(cubeviz_helper, spectrum1d_cube_custom_ # and that calculated moment has the correct units mm.calculate_moment() assert mm.moment.unit == moment_unit - - # TODO: All this below can be removed when we remove SB->flux unit support from Moment Maps - - uc.flux_or_sb.selected = 'Flux' - mm._set_data_units() - - # and make sure this change is propagated - output_unit_moment_0 = mm.output_unit_items[0] - assert output_unit_moment_0['label'] == 'Flux' - assert output_unit_moment_0['unit_str'] == 'Jy' - - mm.calculate_moment() - assert mm.moment.unit == moment_unit_no_sr diff --git a/jdaviz/utils.py b/jdaviz/utils.py index 972256e81a..93e8c75104 100644 --- a/jdaviz/utils.py +++ b/jdaviz/utils.py @@ -360,9 +360,7 @@ def converter_flux(x): # Surface Brightness -> Flux def iconverter_flux(x): # Flux -> Surface Brightness return x / pixar_sr - return [(u.MJy / u.sr, u.MJy, converter_flux, iconverter_flux), - # TODO: This can be removed when we remove SB->flux unit support from Moment Maps - (u.MJy * u.m / u.sr, u.MJy * u.m, converter_flux, iconverter_flux)] + return [(u.MJy / u.sr, u.MJy, converter_flux, iconverter_flux)] def spectral_axis_conversion(values, original_units, target_units): From d7686f4890c07fae7cee8e3986d689a33b4627a7 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 12 Jul 2024 09:38:07 -0400 Subject: [PATCH 09/29] add os import --- jdaviz/tests/test_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/jdaviz/tests/test_utils.py b/jdaviz/tests/test_utils.py index bbb49300cd..2a4019aeaa 100644 --- a/jdaviz/tests/test_utils.py +++ b/jdaviz/tests/test_utils.py @@ -1,4 +1,5 @@ import os +import pytest import warnings import photutils From e99058a08d25c17ccbfa8fa252489a3399269fe2 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 12 Jul 2024 09:54:47 -0400 Subject: [PATCH 10/29] remove duplicate import from rebase --- jdaviz/tests/test_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jdaviz/tests/test_utils.py b/jdaviz/tests/test_utils.py index 2a4019aeaa..38bf8efc66 100644 --- a/jdaviz/tests/test_utils.py +++ b/jdaviz/tests/test_utils.py @@ -3,7 +3,6 @@ import warnings import photutils -import pytest from asdf.exceptions import AsdfWarning from astropy import units as u from astropy.utils import minversion From fa7c9a58a53334a2c61e2559835813846458207b Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 12 Jul 2024 16:39:43 -0400 Subject: [PATCH 11/29] reconcile tests, change hints/docs, handle dimensionless case --- jdaviz/configs/imviz/plugins/coords_info/coords_info.py | 2 +- .../specviz/plugins/unit_conversion/unit_conversion.py | 9 ++++----- .../specviz/plugins/unit_conversion/unit_conversion.vue | 4 ++-- jdaviz/utils.py | 2 ++ 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index b1306b198d..954963a8f7 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -562,7 +562,7 @@ def _copy_axes_to_spectral(): if '_pixel_scale_factor' in sp.meta: eqv = [(u.MJy / u.sr, u.MJy, - lambda x: (x * np.array(sp.meta.get('_pixel_scale_factor', 1))), + lambda x: (x * np.asarray(sp.meta.get('_pixel_scale_factor', 1))), lambda x: x)] eqv += u.spectral_density(sp.spectral_axis) disp_flux = sp.flux.to_value(viewer.state.y_display_unit, eqv) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 2ce0150742..8713e489d6 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -47,11 +47,11 @@ class UnitConversion(PluginTemplateMixin): * ``spectral_unit`` (:class:`~jdaviz.core.template_mixin.UnitSelectPluginComponent`): Global unit to use for all spectral axes. * ``flux_or_sb`` (:class:`~jdaviz.core.template_mixin.SelectPluginComponent`): - Y-axis physical type selection. Currently only accessible in Cubeviz. + Select the y-axis physical type for the spectrum-viewer. * ``flux_unit`` (:class:`~jdaviz.core.template_mixin.UnitSelectPluginComponent`): - Global flux unit to use for y-axis. + Global display unit for flux axis. * ``sb_unit`` (:class:`~jdaviz.core.template_mixin.UnitSelectPluginComponent`): - Global surface brightness unit to use for y-axis. + Global display unit for surface brightness axis. """ template_file = __file__, "unit_conversion.vue" @@ -148,7 +148,6 @@ def _on_glue_y_display_unit_changed(self, y_unit): return self.spectrum_viewer.set_plot_axes() - flux_or_sb = '' if check_if_unit_is_per_solid_angle(y_unit): flux_or_sb = 'Surface Brightness' else: @@ -168,7 +167,7 @@ def _on_glue_y_display_unit_changed(self, y_unit): if self.app.config == 'cubeviz': sb_choices = create_sb_equivalencies_list(y_u / u.sr, x_u) self.sb_unit.choices = sb_choices - if not self.sb_unit.selected: + if not self.sb_unit.selected and self.flux_unit.selected: self.sb_unit.selected = y_unit + " / sr" self.flux_unit.choices = flux_choices diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index 8b28bc021e..beed74f1fc 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -26,7 +26,7 @@ :items="flux_unit_items.map(i => i.label)" v-model="flux_unit_selected" label="Flux Unit" - hint="Global display unit for y-axis axis." + hint="Global display unit for flux axis." persistent-hint > @@ -38,7 +38,7 @@ :items="sb_unit_items.map(i => i.label)" v-model="sb_unit_selected" label="Surface Brightness Unit" - hint="Global display unit for y-axis axis." + hint="Global display unit for surface brightness axis." persistent-hint :disabled="!can_translate" > diff --git a/jdaviz/utils.py b/jdaviz/utils.py index 93e8c75104..46d3e4fbca 100644 --- a/jdaviz/utils.py +++ b/jdaviz/utils.py @@ -312,6 +312,8 @@ def flux_conversion(spec, values, original_units, target_units): result : float array Flux values in the target units. """ + if not target_units: + target_units = original_units # If there are only two values, this is likely the limits being converted, so then # in case we need to use the spectral density equivalency, we need to provide only # to spectral axis values. If there is only one value From da0a96791aab842ca2138370bf491b0c8180d177 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Mon, 15 Jul 2024 15:34:24 -0400 Subject: [PATCH 12/29] emit GlobalDisplayUnitChanged msg when toggling SB<>flux --- .../configs/specviz/plugins/unit_conversion/unit_conversion.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 8713e489d6..51d447ba42 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -207,6 +207,9 @@ def translate_units(self, flux_or_sb_selected): self.spectrum_viewer.state.y_display_unit = str(spec_units) self.flux_or_sb.selected = 'Surface Brightness' + self.hub.broadcast(GlobalDisplayUnitChanged('flux', + spec_units, + sender=self)) self.spectrum_viewer.reset_limits() @observe('spectral_unit_selected') From 0c96e2e01ef30fdb6ee497a244049580c6fb7d73 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 16 Jul 2024 16:47:11 -0400 Subject: [PATCH 13/29] resolve most tests --- .../configs/specviz/plugins/unit_conversion/unit_conversion.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 51d447ba42..fac6ef9bf7 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -167,11 +167,10 @@ def _on_glue_y_display_unit_changed(self, y_unit): if self.app.config == 'cubeviz': sb_choices = create_sb_equivalencies_list(y_u / u.sr, x_u) self.sb_unit.choices = sb_choices - if not self.sb_unit.selected and self.flux_unit.selected: - self.sb_unit.selected = y_unit + " / sr" self.flux_unit.choices = flux_choices self.flux_unit.selected = y_unit + self.flux_or_sb.selected = 'Flux' elif flux_or_sb == 'Surface Brightness' and y_unit != self.sb_unit.selected: sb_choices = create_sb_equivalencies_list(y_u, x_u) From 72295cf2c45bea7d2e919d3b7e06f4e490b96ef2 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 17 Jul 2024 10:23:31 -0400 Subject: [PATCH 14/29] temporarily remove translation message --- .../unit_conversion/unit_conversion.py | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index fac6ef9bf7..15a4b3881a 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -167,6 +167,8 @@ def _on_glue_y_display_unit_changed(self, y_unit): if self.app.config == 'cubeviz': sb_choices = create_sb_equivalencies_list(y_u / u.sr, x_u) self.sb_unit.choices = sb_choices + # if not self.sb_unit.selected: + # self.sb_unit.selected = y_unit + ' / sr' self.flux_unit.choices = flux_choices self.flux_unit.selected = y_unit @@ -206,9 +208,9 @@ def translate_units(self, flux_or_sb_selected): self.spectrum_viewer.state.y_display_unit = str(spec_units) self.flux_or_sb.selected = 'Surface Brightness' - self.hub.broadcast(GlobalDisplayUnitChanged('flux', - spec_units, - sender=self)) + # self.hub.broadcast(GlobalDisplayUnitChanged('flux', + # spec_units, + # sender=self)) self.spectrum_viewer.reset_limits() @observe('spectral_unit_selected') @@ -227,7 +229,7 @@ def _on_flux_unit_changed(self, *args): data_collection_unit = '' # need to determine the input spectrum units to disable the additional - # drop down and possiblity of translations + # drop down and possiblity of translations in Specviz. if self.app.data_collection[0] and self.app.config == 'specviz': if check_if_unit_is_per_solid_angle(self.app.data_collection[0].get_object()._unit): data_collection_unit = 'Surface Brightness' @@ -237,8 +239,11 @@ def _on_flux_unit_changed(self, *args): data_collection_unit = 'Flux' for arg in args: - # determine if flux or surface brightness unit was changed by user + # determine if flux or surface brightness unit was changed by user if arg['name'] == 'flux_unit_selected': + # 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. if data_collection_unit == 'Surface Brightness': raise ValueError( "Unit translation between Flux and Surface Brightness " @@ -250,9 +255,9 @@ def _on_flux_unit_changed(self, *args): self.flux_or_sb.selected = 'Flux' untranslatable_units = self.untranslatable_units() - # disable translator is flux unit is untranslatable - # still can select surface brightness units, just disables - # flux -> surface brightnes of particular unit selected. + # disable translator if flux unit is untranslatable, + # still can convert flux units, this just disables flux + # to surface brightnes translation for units in list. if flux_or_sb in untranslatable_units: self.can_translate = False else: @@ -260,6 +265,9 @@ def _on_flux_unit_changed(self, *args): elif arg['name'] == 'sb_unit_selected': if data_collection_unit == 'Flux': + # when the configuration is Specviz, translation is not currently supported. + # If in Cubeviz, all spectra pass through Spectral Xxtraction plugin and will + # have a scale factor assigned in the metadata, enabling translation. raise ValueError( "Unit translation between Flux and Surface Brightness " "is not supported in Specviz." From aa718e3dbc18e03d4568c8ca70c3d8ea589b97ed Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 17 Jul 2024 14:10:28 -0400 Subject: [PATCH 15/29] add comments, reenable global display message, resolve tests --- .../unit_conversion/unit_conversion.py | 23 +++++++++++++++---- jdaviz/utils.py | 2 ++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 15a4b3881a..a65c566197 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -167,8 +167,8 @@ def _on_glue_y_display_unit_changed(self, y_unit): if self.app.config == 'cubeviz': sb_choices = create_sb_equivalencies_list(y_u / u.sr, x_u) self.sb_unit.choices = sb_choices - # if not self.sb_unit.selected: - # self.sb_unit.selected = y_unit + ' / sr' + if y_unit + ' / sr' in self.sb_unit.choices: + self.sb_unit.selected = y_unit + ' / sr' self.flux_unit.choices = flux_choices self.flux_unit.selected = y_unit @@ -193,6 +193,10 @@ def translate_units(self, flux_or_sb_selected): 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_selected == 'Flux': spec_units *= u.sr @@ -207,10 +211,13 @@ def translate_units(self, flux_or_sb_selected): # update display units self.spectrum_viewer.state.y_display_unit = str(spec_units) self.flux_or_sb.selected = 'Surface Brightness' + # entered the translator when we shouldn't translate + else: + return - # self.hub.broadcast(GlobalDisplayUnitChanged('flux', - # spec_units, - # sender=self)) + self.hub.broadcast(GlobalDisplayUnitChanged('flux', + spec_units, + sender=self)) self.spectrum_viewer.reset_limits() @observe('spectral_unit_selected') @@ -277,9 +284,15 @@ def _on_flux_unit_changed(self, *args): # update flux or surface brightness dropdown if necessary if not check_if_unit_is_per_solid_angle(current_y): self.flux_or_sb.selected = 'Surface Brightness' + else: + return yunit = _valid_glue_display_unit(flux_or_sb, self.spectrum_viewer, 'y') + # on instantiation, we set determine flux choices and selection + # after surface brightness + if not self.flux_unit.choices: + return if self.spectrum_viewer.state.y_display_unit != yunit: self.spectrum_viewer.state.y_display_unit = yunit self.hub.broadcast(GlobalDisplayUnitChanged('flux', diff --git a/jdaviz/utils.py b/jdaviz/utils.py index 46d3e4fbca..53cc5a8a03 100644 --- a/jdaviz/utils.py +++ b/jdaviz/utils.py @@ -312,6 +312,8 @@ def flux_conversion(spec, values, original_units, target_units): result : float array Flux values in the target units. """ + # we set surface brightness choices and selection before flux, which can + # cause a dimensionless translation attempt at instantiation if not target_units: target_units = original_units # If there are only two values, this is likely the limits being converted, so then From f5b1f482d1514b3076b23a723e9efc1a9a03c7d9 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 17 Jul 2024 14:54:41 -0400 Subject: [PATCH 16/29] force sb unit for line flux test --- .../specviz/plugins/line_analysis/line_analysis.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py index 5fea5bb25e..5eaf2e6809 100644 --- a/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py +++ b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py @@ -288,6 +288,10 @@ def _uncertainty(result): # don't need these if statements if function == "Line Flux": flux_unit = spec_subtracted.flux.unit + add_flux = False + if str(flux_unit) == '': + add_flux = True + flux_unit = u.Unit(self.spectrum_viewer.state.y_display_unit) # If the flux unit is equivalent to Jy, or Jy per spaxel for Cubeviz, # enforce integration in frequency space if (flux_unit.is_equivalent(u.Jy) or @@ -300,7 +304,10 @@ def _uncertainty(result): uncertainty=spec_subtracted.uncertainty) try: - raw_result = analysis.line_flux(freq_spec) + if add_flux: + raw_result = analysis.line_flux(freq_spec) * flux_unit + else: + raw_result = analysis.line_flux(freq_spec) except ValueError as e: # can happen if interpolation out-of-bounds or any error from specutils # let's avoid the whole app crashing and instead expose the error to the @@ -332,7 +339,10 @@ def _uncertainty(result): flux=spec_subtracted.flux, uncertainty=spec_subtracted.uncertainty) try: - raw_result = analysis.line_flux(wave_spec) + if add_flux: + raw_result = analysis.line_flux(wave_spec) * flux_unit + else: + raw_result = raw_result = analysis.line_flux(wave_spec) except ValueError as e: # can happen if interpolation out-of-bounds or any error from specutils # let's avoid the whole app crashing and instead expose the error to the From 79b14d4fd5bd82365e9fa2b0ebd5d359868dacf4 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 18 Jul 2024 12:39:52 -0400 Subject: [PATCH 17/29] add comments, clarify naming, add case handling for args --- CHANGES.rst | 2 +- .../unit_conversion/unit_conversion.py | 35 ++++++++++++------- .../unit_conversion/unit_conversion.vue | 4 +-- jdaviz/utils.py | 2 +- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index fb73de5a42..26efbd097b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,7 @@ New Features ------------ - Added flux/surface brightness translation and surface brightness - unit conversion in Cubeviz and Specviz. [#2781] + unit conversion in Cubeviz and Specviz. [#2781, #2940] - Plugin tray is now open by default. [#2892] diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index a65c566197..4789c821d7 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -67,7 +67,9 @@ class UnitConversion(PluginTemplateMixin): flux_or_sb_items = List().tag(sync=True) flux_or_sb_selected = Unicode().tag(sync=True) - specviz_disabler = Unicode().tag(sync=True) + # in certain configs, a pixel scale factor will not be in the FITS header + # we need to disable translation in the API and UI variables/functions. + flux_or_sb_config_disabler = Unicode().tag(sync=True) can_translate = Bool(True).tag(sync=True) def __init__(self, *args, **kwargs): @@ -110,11 +112,11 @@ def __init__(self, *args, **kwargs): def user_api(self): if self.app.config == 'cubeviz': return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_unit', 'sb_unit')) # noqa - elif self.app.config == 'specviz' and not self.specviz_disabler: + if self.app.config == 'specviz' and not self.flux_or_sb_config_disabler: return PluginUserApi(self, expose=('spectral_unit', 'flux_unit', 'sb_unit')) - if self.specviz_disabler == 'Flux': + if self.flux_or_sb_config_disabler == 'Flux': return PluginUserApi(self, expose=('spectral_unit', 'sb_unit')) - if self.specviz_disabler == 'Surface Brightness': + if self.flux_or_sb_config_disabler == 'Surface Brightness': return PluginUserApi(self, expose=('spectral_unit', 'flux_unit')) def _on_glue_x_display_unit_changed(self, x_unit): @@ -240,10 +242,10 @@ def _on_flux_unit_changed(self, *args): if self.app.data_collection[0] and self.app.config == 'specviz': if check_if_unit_is_per_solid_angle(self.app.data_collection[0].get_object()._unit): data_collection_unit = 'Surface Brightness' - self.specviz_disabler = 'Flux' + self.flux_or_sb_config_disabler = 'Flux' else: - self.specviz_disabler = 'Surface Brightness' data_collection_unit = 'Flux' + self.flux_or_sb_config_disabler = 'Surface Brightness' for arg in args: # determine if flux or surface brightness unit was changed by user @@ -253,8 +255,8 @@ def _on_flux_unit_changed(self, *args): # have a scale factor assigned in the metadata, enabling translation. if data_collection_unit == 'Surface Brightness': raise ValueError( - "Unit translation between Flux and Surface Brightness " - "is not supported in Specviz." + f"Unit translation between Flux and Surface Brightness " + f"is not supported in {self.app.config}." ) flux_or_sb = self.flux_unit.selected # update flux or surface brightness dropdown if necessary @@ -276,8 +278,8 @@ def _on_flux_unit_changed(self, *args): # If in Cubeviz, all spectra pass through Spectral Xxtraction plugin and will # have a scale factor assigned in the metadata, enabling translation. raise ValueError( - "Unit translation between Flux and Surface Brightness " - "is not supported in Specviz." + f"Unit translation between Flux and Surface Brightness " + f"is not supported in {self.app.config}." ) flux_or_sb = self.sb_unit.selected self.can_translate = True @@ -312,17 +314,24 @@ def _translate(self, *args): # need to check current y_unit to see if we need to translate y_unit = self.spectrum_viewer.state.y_display_unit for arg in args: - if arg['name'] == 'flux_unit_selected': + # skip if arg is not a dictionary + if not isinstance(arg, dict): + continue + # skip if 'name' key is missing + name = arg.get('name') + if not name: + continue + if name == 'flux_unit_selected': # don't translate if self.flux_or_sb == Flux if not check_if_unit_is_per_solid_angle(y_unit): return flux_or_sb = 'Flux' - elif arg['name'] == 'sb_unit_selected': + elif name == 'sb_unit_selected': # don't translate if self.flux_or_sb == Surface Brightness if check_if_unit_is_per_solid_angle(y_unit): return flux_or_sb = 'Surface Brightness' - elif arg['name'] == 'flux_or_sb_selected': + elif name == 'flux_or_sb_selected': flux_or_sb = self.flux_or_sb_selected else: return diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index beed74f1fc..4f3c9e2e22 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -19,7 +19,7 @@ > - + - + Date: Thu, 18 Jul 2024 13:18:57 -0400 Subject: [PATCH 18/29] remove args and loop, replaced with msg --- .../unit_conversion/unit_conversion.py | 132 +++++++++--------- 1 file changed, 69 insertions(+), 63 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 4789c821d7..14a4e1fc2b 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -232,7 +232,7 @@ def _on_spectral_unit_changed(self, *args): sender=self)) @observe('flux_unit_selected', 'sb_unit_selected') - def _on_flux_unit_changed(self, *args): + def _on_flux_unit_changed(self, msg): flux_or_sb = None current_y = self.spectrum_viewer.state.y_display_unit @@ -247,47 +247,53 @@ def _on_flux_unit_changed(self, *args): data_collection_unit = 'Flux' self.flux_or_sb_config_disabler = 'Surface Brightness' - for arg in args: - # determine if flux or surface brightness unit was changed by user - if arg['name'] == 'flux_unit_selected': + # skip if arg is not a dictionary + if not isinstance(msg, dict): + raise TypeError("Expected msg to be a dictionary") + # skip if 'name' key is missing + name = msg.get('name') + if not name: + raise KeyError("Expected 'name' key in msg") + + # determine if flux or surface brightness unit was changed by user + if name == 'flux_unit_selected': + # 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. + if data_collection_unit == 'Surface Brightness': + raise ValueError( + f"Unit translation between Flux and Surface Brightness " + f"is not supported in {self.app.config}." + ) + flux_or_sb = self.flux_unit.selected + # update flux or surface brightness dropdown if necessary + if check_if_unit_is_per_solid_angle(current_y): + self.flux_or_sb.selected = 'Flux' + untranslatable_units = self.untranslatable_units() + # disable translator if flux unit is untranslatable, + # still can convert flux units, this just disables flux + # to surface brightnes translation for units in list. + if flux_or_sb in untranslatable_units: + self.can_translate = False + else: + self.can_translate = True + + elif name == 'sb_unit_selected': + if data_collection_unit == 'Flux': # when the configuration is Specviz, translation is not currently supported. - # If in Cubeviz, all spectra pass through Spectral Extraction plugin and will + # If in Cubeviz, all spectra pass through Spectral Xxtraction plugin and will # have a scale factor assigned in the metadata, enabling translation. - if data_collection_unit == 'Surface Brightness': - raise ValueError( - f"Unit translation between Flux and Surface Brightness " - f"is not supported in {self.app.config}." - ) - flux_or_sb = self.flux_unit.selected - # update flux or surface brightness dropdown if necessary - if check_if_unit_is_per_solid_angle(current_y): - self.flux_or_sb.selected = 'Flux' - - untranslatable_units = self.untranslatable_units() - # disable translator if flux unit is untranslatable, - # still can convert flux units, this just disables flux - # to surface brightnes translation for units in list. - if flux_or_sb in untranslatable_units: - self.can_translate = False - else: - self.can_translate = True - - elif arg['name'] == 'sb_unit_selected': - if data_collection_unit == 'Flux': - # when the configuration is Specviz, translation is not currently supported. - # If in Cubeviz, all spectra pass through Spectral Xxtraction plugin and will - # have a scale factor assigned in the metadata, enabling translation. - raise ValueError( - f"Unit translation between Flux and Surface Brightness " - f"is not supported in {self.app.config}." - ) - flux_or_sb = self.sb_unit.selected - self.can_translate = True - # update flux or surface brightness dropdown if necessary - if not check_if_unit_is_per_solid_angle(current_y): - self.flux_or_sb.selected = 'Surface Brightness' - else: - return + raise ValueError( + f"Unit translation between Flux and Surface Brightness " + f"is not supported in {self.app.config}." + ) + flux_or_sb = self.sb_unit.selected + self.can_translate = True + # update flux or surface brightness dropdown if necessary + if not check_if_unit_is_per_solid_angle(current_y): + self.flux_or_sb.selected = 'Surface Brightness' + else: + return yunit = _valid_glue_display_unit(flux_or_sb, self.spectrum_viewer, 'y') @@ -303,38 +309,38 @@ def _on_flux_unit_changed(self, *args): self.spectrum_viewer.reset_limits() @observe('flux_or_sb_selected', 'flux_unit_selected', 'sb_unit_selected') - def _translate(self, *args): + def _translate(self, msg): # currently unsupported, can be supported with a scale factor if self.app.config == 'specviz': return flux_or_sb = None - if args: + if msg: # need to check current y_unit to see if we need to translate y_unit = self.spectrum_viewer.state.y_display_unit - for arg in args: - # skip if arg is not a dictionary - if not isinstance(arg, dict): - continue - # skip if 'name' key is missing - name = arg.get('name') - if not name: - continue - if name == 'flux_unit_selected': - # don't translate if self.flux_or_sb == Flux - if not check_if_unit_is_per_solid_angle(y_unit): - return - flux_or_sb = 'Flux' - elif name == 'sb_unit_selected': - # don't translate if self.flux_or_sb == Surface Brightness - if check_if_unit_is_per_solid_angle(y_unit): - return - flux_or_sb = 'Surface Brightness' - elif name == 'flux_or_sb_selected': - flux_or_sb = self.flux_or_sb_selected - else: + + # skip if arg is not a dictionary + if not isinstance(msg, dict): + raise TypeError("Expected msg to be a dictionary") + # skip if 'name' key is missing + name = msg.get('name') + if not name: + raise KeyError("Expected 'name' key in msg") + if name == 'flux_unit_selected': + # don't translate if self.flux_or_sb == Flux + if not check_if_unit_is_per_solid_angle(y_unit): + return + flux_or_sb = 'Flux' + elif name == 'sb_unit_selected': + # don't translate if self.flux_or_sb == Surface Brightness + if check_if_unit_is_per_solid_angle(y_unit): return + flux_or_sb = 'Surface Brightness' + elif name == 'flux_or_sb_selected': + flux_or_sb = self.flux_or_sb_selected + else: + return # we want to raise an error if a user tries to translate with an # untranslated Flux unit using the API From 161b3fbc37ece676b74a3ff06b24dc0bdb544e32 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 18 Jul 2024 14:40:10 -0400 Subject: [PATCH 19/29] consolidate 2 translator functions into 1, consolidate _translate and _on_flux_unit_changed --- .../unit_conversion/unit_conversion.py | 111 +++++++----------- 1 file changed, 42 insertions(+), 69 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 14a4e1fc2b..83149eb332 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -190,38 +190,6 @@ def _on_glue_y_display_unit_changed(self, y_unit): self.sb_unit.choices = sb_choices self.sb_unit.selected = y_unit - def translate_units(self, flux_or_sb_selected): - if self.spectrum_viewer.state.y_display_unit: - 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_selected == 'Flux': - spec_units *= u.sr - # update display units - self.spectrum_viewer.state.y_display_unit = str(spec_units) - self.flux_or_sb.selected = 'Flux' - - # Flux -> Surface Brightness - elif (not check_if_unit_is_per_solid_angle(spec_units) - and flux_or_sb_selected == 'Surface Brightness'): - spec_units /= u.sr - # update display units - self.spectrum_viewer.state.y_display_unit = str(spec_units) - self.flux_or_sb.selected = 'Surface Brightness' - # entered the translator when we shouldn't translate - else: - return - - self.hub.broadcast(GlobalDisplayUnitChanged('flux', - spec_units, - sender=self)) - self.spectrum_viewer.reset_limits() - @observe('spectral_unit_selected') def _on_spectral_unit_changed(self, *args): xunit = _valid_glue_display_unit(self.spectral_unit.selected, self.spectrum_viewer, 'x') @@ -231,7 +199,7 @@ def _on_spectral_unit_changed(self, *args): self.spectral_unit.selected, sender=self)) - @observe('flux_unit_selected', 'sb_unit_selected') + @observe('flux_or_sb_selected', 'flux_unit_selected', 'sb_unit_selected') def _on_flux_unit_changed(self, msg): flux_or_sb = None current_y = self.spectrum_viewer.state.y_display_unit @@ -239,7 +207,11 @@ def _on_flux_unit_changed(self, msg): data_collection_unit = '' # need to determine the input spectrum units to disable the additional # drop down and possiblity of translations in Specviz. - if self.app.data_collection[0] and self.app.config == 'specviz': + if ( + len(self.app.data_collection) > 0 and + self.app.data_collection[0] and + self.app.config == 'specviz' + ): if check_if_unit_is_per_solid_angle(self.app.data_collection[0].get_object()._unit): data_collection_unit = 'Surface Brightness' self.flux_or_sb_config_disabler = 'Flux' @@ -268,6 +240,7 @@ def _on_flux_unit_changed(self, msg): flux_or_sb = self.flux_unit.selected # update flux or surface brightness dropdown if necessary if check_if_unit_is_per_solid_angle(current_y): + self._translate('Flux') self.flux_or_sb.selected = 'Flux' untranslatable_units = self.untranslatable_units() # disable translator if flux unit is untranslatable, @@ -291,16 +264,16 @@ def _on_flux_unit_changed(self, msg): self.can_translate = True # update flux or surface brightness dropdown if necessary if not check_if_unit_is_per_solid_angle(current_y): + self._translate('Surface Brightness') self.flux_or_sb.selected = 'Surface Brightness' + elif name == 'flux_or_sb_selected': + self._translate(self.flux_or_sb_selected) + return else: return yunit = _valid_glue_display_unit(flux_or_sb, self.spectrum_viewer, 'y') - # on instantiation, we set determine flux choices and selection - # after surface brightness - if not self.flux_unit.choices: - return if self.spectrum_viewer.state.y_display_unit != yunit: self.spectrum_viewer.state.y_display_unit = yunit self.hub.broadcast(GlobalDisplayUnitChanged('flux', @@ -308,40 +281,11 @@ def _on_flux_unit_changed(self, msg): sender=self)) self.spectrum_viewer.reset_limits() - @observe('flux_or_sb_selected', 'flux_unit_selected', 'sb_unit_selected') - def _translate(self, msg): + def _translate(self, flux_or_sb=None): # currently unsupported, can be supported with a scale factor if self.app.config == 'specviz': return - flux_or_sb = None - - if msg: - # need to check current y_unit to see if we need to translate - y_unit = self.spectrum_viewer.state.y_display_unit - - # skip if arg is not a dictionary - if not isinstance(msg, dict): - raise TypeError("Expected msg to be a dictionary") - # skip if 'name' key is missing - name = msg.get('name') - if not name: - raise KeyError("Expected 'name' key in msg") - if name == 'flux_unit_selected': - # don't translate if self.flux_or_sb == Flux - if not check_if_unit_is_per_solid_angle(y_unit): - return - flux_or_sb = 'Flux' - elif name == 'sb_unit_selected': - # don't translate if self.flux_or_sb == Surface Brightness - if check_if_unit_is_per_solid_angle(y_unit): - return - flux_or_sb = 'Surface Brightness' - elif name == 'flux_or_sb_selected': - flux_or_sb = self.flux_or_sb_selected - else: - return - # we want to raise an error if a user tries to translate with an # untranslated Flux unit using the API untranslatable_units = self.untranslatable_units() @@ -355,7 +299,36 @@ def _translate(self, msg): f"that is not in the following list: {untranslatable_units}." ) - self.translate_units(flux_or_sb) + if self.spectrum_viewer.state.y_display_unit: + 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 + # update display units + self.spectrum_viewer.state.y_display_unit = str(spec_units) + self.flux_or_sb.selected = 'Flux' + + # Flux -> Surface Brightness + elif (not check_if_unit_is_per_solid_angle(spec_units) + and flux_or_sb == 'Surface Brightness'): + spec_units /= u.sr + # update display units + self.spectrum_viewer.state.y_display_unit = str(spec_units) + self.flux_or_sb.selected = 'Surface Brightness' + # entered the translator when we shouldn't translate + else: + return + + self.hub.broadcast(GlobalDisplayUnitChanged('flux', + spec_units, + sender=self)) + self.spectrum_viewer.reset_limits() def untranslatable_units(self): untranslatable_units = [ From 119784a6a6b23fc61cfecac7963f229e5a6a9bd8 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 18 Jul 2024 14:59:35 -0400 Subject: [PATCH 20/29] address test failures --- .../configs/specviz/plugins/unit_conversion/unit_conversion.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 83149eb332..c173e51adf 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -273,6 +273,9 @@ def _on_flux_unit_changed(self, msg): return yunit = _valid_glue_display_unit(flux_or_sb, self.spectrum_viewer, 'y') + # may need to be updated if translations in other configs going to be supported + if not self.flux_unit.choices and self.app.config == 'cubeviz': + return if self.spectrum_viewer.state.y_display_unit != yunit: self.spectrum_viewer.state.y_display_unit = yunit From 4c8e667a195df91412b7f55f8fb80dea2034f0bc Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 18 Jul 2024 16:20:47 -0400 Subject: [PATCH 21/29] update marks.py equivalencies, resolving UnitConversion errors --- jdaviz/core/marks.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 0286c0dac9..935a1a0481 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -5,6 +5,7 @@ from bqplot.marks import Lines, Label, Scatter from glue.core import HubListener from specutils import Spectrum1D +from jdaviz.utils import _eqv_pixar_sr from jdaviz.core.events import GlobalDisplayUnitChanged from jdaviz.core.events import (SliceToolStateMessage, LineIdentifyMessage, @@ -112,7 +113,11 @@ def set_y_unit(self, unit=None): if self.viewer.default_class is Spectrum1D: spec = self.viewer.state.reference_data.get_object(cls=Spectrum1D) eqv = u.spectral_density(spec.spectral_axis) - y = (self.y * self.yunit).to_value(unit, equivalencies=eqv) + if ('_pixel_scale_factor' in spec.meta): + eqv = _eqv_pixar_sr(spec.meta['_pixel_scale_factor']) + y = (self.y * self.yunit).to_value(unit, equivalencies=eqv) + else: + y = (self.y * self.yunit).to_value(unit, equivalencies=eqv) else: y = (self.y * self.yunit).to_value(unit) self.yunit = unit From ac1ddc8445a3f43d5f57c4ba2da7d55c818fbaba Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 18 Jul 2024 16:49:44 -0400 Subject: [PATCH 22/29] make sure not only translation equivalencies are in marks.py but also conversions --- jdaviz/core/marks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 935a1a0481..5c46e6a7fb 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -114,7 +114,7 @@ def set_y_unit(self, unit=None): spec = self.viewer.state.reference_data.get_object(cls=Spectrum1D) eqv = u.spectral_density(spec.spectral_axis) if ('_pixel_scale_factor' in spec.meta): - eqv = _eqv_pixar_sr(spec.meta['_pixel_scale_factor']) + eqv += _eqv_pixar_sr(spec.meta['_pixel_scale_factor']) y = (self.y * self.yunit).to_value(unit, equivalencies=eqv) else: y = (self.y * self.yunit).to_value(unit, equivalencies=eqv) From c62569b08bb7b1d19aca417aef9a054e4917de70 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 18 Jul 2024 18:18:21 -0400 Subject: [PATCH 23/29] add test coverage --- .../tests/test_unit_conversion.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py index 59648060f8..2db86a8526 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py @@ -146,7 +146,6 @@ def test_unit_translation(cubeviz_helper): # When the dropdown is displayed, this ensures the loaded # data collection item units will be used for translations. - uc_plg._obj.show_translator = True assert uc_plg._obj.flux_or_sb_selected == 'Flux' # to have access to display units @@ -178,11 +177,15 @@ def test_sb_unit_conversion(cubeviz_helper): uc_plg = cubeviz_helper.plugins['Unit Conversion'] uc_plg.open_in_tray() + # ensure that per solid angle cube defaults to Flux spectrum + assert uc_plg.flux_or_sb == 'Flux' + # flux choices is populated with flux units + assert uc_plg.flux_unit.choices + # to have access to display units viewer_1d = cubeviz_helper.app.get_viewer( cubeviz_helper._default_spectrum_viewer_reference_name) - uc_plg._obj.show_translator = True uc_plg.flux_or_sb.selected = 'Surface Brightness' # Surface Brightness conversion @@ -201,7 +204,6 @@ def test_sb_unit_conversion(cubeviz_helper): uc_plg.sb_unit = 'MJy / sr' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) - uc_plg._obj.show_translator = True uc_plg._obj.flux_or_sb_selected = 'Flux' uc_plg.flux_unit = 'MJy' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) @@ -210,3 +212,13 @@ def test_sb_unit_conversion(cubeviz_helper): la = cubeviz_helper.plugins['Line Analysis']._obj assert la.dataset.get_selected_spectrum(use_display_units=True) + + +def test_translation_disabled_without_pixar_sr(specviz_helper, spectrum1d): + specviz_helper.load_data(spectrum1d, data_label="Test 1D Spectrum") + + uc_plg = specviz_helper.plugins['Unit Conversion'] + # spectrum loaded is in Flux units, make sure sb_units don't + # display in the API and exposed translation isn't possible + assert not hasattr(uc_plg, 'sb_unit') + assert not hasattr(uc_plg, 'flux_or_sb') From c385afff88640d7a7a42c0c71020641fba7f4179 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 19 Jul 2024 09:32:18 -0400 Subject: [PATCH 24/29] use astropy units, small logic tweaks, cache untranslatable units --- .../plugins/line_analysis/line_analysis.py | 5 ++-- .../unit_conversion/unit_conversion.py | 24 +++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py index 5eaf2e6809..6704a388a1 100644 --- a/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py +++ b/jdaviz/configs/specviz/plugins/line_analysis/line_analysis.py @@ -288,10 +288,11 @@ def _uncertainty(result): # don't need these if statements if function == "Line Flux": flux_unit = spec_subtracted.flux.unit - add_flux = False - if str(flux_unit) == '': + if flux_unit == u.dimensionless_unscaled: add_flux = True flux_unit = u.Unit(self.spectrum_viewer.state.y_display_unit) + else: + add_flux = False # If the flux unit is equivalent to Jy, or Jy per spaxel for Cubeviz, # enforce integration in frequency space if (flux_unit.is_equivalent(u.Jy) or diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index c173e51adf..76d1739c4c 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -242,7 +242,7 @@ def _on_flux_unit_changed(self, msg): if check_if_unit_is_per_solid_angle(current_y): self._translate('Flux') self.flux_or_sb.selected = 'Flux' - untranslatable_units = self.untranslatable_units() + untranslatable_units = self._untranslatable_units # disable translator if flux unit is untranslatable, # still can convert flux units, this just disables flux # to surface brightnes translation for units in list. @@ -291,7 +291,7 @@ def _translate(self, flux_or_sb=None): # we want to raise an error if a user tries to translate with an # untranslated Flux unit using the API - untranslatable_units = self.untranslatable_units() + untranslatable_units = self._untranslatable_units untranslatable_units = units_to_strings(untranslatable_units) if hasattr(self, 'flux_unit'): @@ -333,13 +333,13 @@ def _translate(self, flux_or_sb=None): sender=self)) self.spectrum_viewer.reset_limits() - def untranslatable_units(self): - untranslatable_units = [ - 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 - ] - return untranslatable_units + @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 + ] From 7be903f54e02546d922cbe2f7c6b41232f23058a Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Mon, 22 Jul 2024 10:22:04 -0400 Subject: [PATCH 25/29] technical review changes --- jdaviz/app.py | 3 -- .../spectral_extraction.py | 2 +- .../imviz/plugins/coords_info/coords_info.py | 7 ++--- .../tests/test_unit_conversion.py | 28 +++++++++++-------- jdaviz/core/marks.py | 2 -- jdaviz/core/validunits.py | 8 ++---- jdaviz/tests/test_utils.py | 7 ++--- 7 files changed, 24 insertions(+), 33 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 806558e74e..de0aacdf72 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -88,9 +88,6 @@ def equivalent_units(self, data, cid, units): 'eV / (Hz s sr m2)', 'erg / (s sr cm2)', 'AB / sr' - # 'erg / (s cm2 Angstrom sr)', 'erg / (Hz s sr cm2)', - # 'ph / (Angstrom s sr cm2)', 'ph / (Hz s sr cm2)', - # 'ST / sr', 'bol / sr' ]) else: # spectral axis # prefer Hz over Bq and um over micron diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py index 0269c6dc7a..f4a76f14fb 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py @@ -502,7 +502,7 @@ def extract(self, return_bg=False, add_data=True, **kwargs): snackbar_message = SnackbarMessage( ("PIXAR_SR FITS header keyword not found when parsing spectral cube. " "Flux/Surface Brightness will use default scale factor of 1."), - color="error", + color="warning", sender=self) self.hub.broadcast(snackbar_message) diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index 954963a8f7..b226a83e4a 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -17,6 +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 __all__ = ['CoordsInfo'] @@ -560,11 +561,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.MJy / u.sr, - u.MJy, - lambda x: (x * np.asarray(sp.meta.get('_pixel_scale_factor', 1))), - lambda x: x)] - eqv += u.spectral_density(sp.spectral_axis) + 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) else: disp_flux = sp.flux.to_value(viewer.state.y_display_unit, diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py index 2db86a8526..b1b5380f2b 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/tests/test_unit_conversion.py @@ -89,14 +89,28 @@ def test_conv_wave_flux(specviz_helper, spectrum1d, uncert): assert u.Unit(viewer.state.y_display_unit) == u.Unit(new_flux) -def test_conv_no_data(specviz_helper): +def test_conv_no_data(specviz_helper, spectrum1d): """plugin unit selections won't have valid choices yet, preventing attempting to set display units.""" plg = specviz_helper.plugins["Unit Conversion"] + # spectrum not load is in Flux units, sb_unit and flux_unit + # should be enabled, flux_or_sb should not be + assert hasattr(plg, 'sb_unit') + assert hasattr(plg, 'flux_unit') + assert not hasattr(plg, 'flux_or_sb') with pytest.raises(ValueError, match="no valid unit choices"): plg.spectral_unit = "micron" assert len(specviz_helper.app.data_collection) == 0 + specviz_helper.load_data(spectrum1d, data_label="Test 1D Spectrum") + plg = specviz_helper.plugins["Unit Conversion"] + + # spectrum loaded in Flux units, make sure sb_units don't + # display in the API and exposed translation isn't possible + assert hasattr(plg, 'flux_unit') + assert not hasattr(plg, 'sb_unit') + assert not hasattr(plg, 'flux_or_sb') + @pytest.mark.skipif(ASTROPY_LT_5_3, reason='this feature relies on astropy v5.3+') def test_non_stddev_uncertainty(specviz_helper): @@ -142,7 +156,7 @@ def test_unit_translation(cubeviz_helper): uc_plg = cubeviz_helper.plugins['Unit Conversion'] # test that the scale factor was set - assert np.all(cubeviz_helper.app.data_collection[-1].meta['_pixel_scale_factor'] != 1) + assert np.all(cubeviz_helper.app.data_collection['Spectrum (sum)'].meta['_pixel_scale_factor'] != 1) # noqa # When the dropdown is displayed, this ensures the loaded # data collection item units will be used for translations. @@ -212,13 +226,3 @@ def test_sb_unit_conversion(cubeviz_helper): la = cubeviz_helper.plugins['Line Analysis']._obj assert la.dataset.get_selected_spectrum(use_display_units=True) - - -def test_translation_disabled_without_pixar_sr(specviz_helper, spectrum1d): - specviz_helper.load_data(spectrum1d, data_label="Test 1D Spectrum") - - uc_plg = specviz_helper.plugins['Unit Conversion'] - # spectrum loaded is in Flux units, make sure sb_units don't - # display in the API and exposed translation isn't possible - assert not hasattr(uc_plg, 'sb_unit') - assert not hasattr(uc_plg, 'flux_or_sb') diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 5c46e6a7fb..ae6533dbdb 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -116,8 +116,6 @@ def set_y_unit(self, unit=None): if ('_pixel_scale_factor' in spec.meta): eqv += _eqv_pixar_sr(spec.meta['_pixel_scale_factor']) y = (self.y * self.yunit).to_value(unit, equivalencies=eqv) - else: - y = (self.y * self.yunit).to_value(unit, equivalencies=eqv) else: y = (self.y * self.yunit).to_value(unit) self.yunit = unit diff --git a/jdaviz/core/validunits.py b/jdaviz/core/validunits.py index 0ff6e9671a..f3cc4b0612 100644 --- a/jdaviz/core/validunits.py +++ b/jdaviz/core/validunits.py @@ -105,15 +105,11 @@ def create_sb_equivalencies_list(sb_unit, spectral_axis_unit): except u.core.UnitConversionError: return [] - locally_defined_sb_units = ['Jy / sr', 'mJy / sr', 'uJy / sr', 'MJy / sr', 'Jy / sr', + locally_defined_sb_units = ['Jy / sr', 'mJy / sr', + 'uJy / sr', 'MJy / sr', 'W / (Hz sr m2)', 'eV / (Hz s sr m2)', 'AB / sr' - # 'erg / (s cm2 sr)', - # 'erg / (s cm2 Angstrom sr)', - # 'erg / (s cm2 Hz sr)', - # 'ph / (Angstrom s sr cm2)', - # 'ph / (s cm2 Hz sr)' ] local_units = [u.Unit(unit) for unit in locally_defined_sb_units] diff --git a/jdaviz/tests/test_utils.py b/jdaviz/tests/test_utils.py index 38bf8efc66..bf4f7ceb2d 100644 --- a/jdaviz/tests/test_utils.py +++ b/jdaviz/tests/test_utils.py @@ -1,8 +1,8 @@ import os -import pytest import warnings import photutils +import pytest from asdf.exceptions import AsdfWarning from astropy import units as u from astropy.utils import minversion @@ -10,7 +10,6 @@ from numpy.testing import assert_allclose from specutils import Spectrum1D -from jdaviz import utils from jdaviz.utils import alpha_index, download_uri_to_path, flux_conversion PHOTUTILS_LT_1_12_1 = not minversion(photutils, "1.12.1.dev") @@ -79,9 +78,9 @@ def test_alpha_index(test_input, expected): def test_alpha_index_exceptions(): with pytest.raises(TypeError, match="index must be an integer"): - utils.alpha_index(4.2) + alpha_index(4.2) with pytest.raises(ValueError, match="index must be positive"): - utils.alpha_index(-1) + alpha_index(-1) def test_uri_to_download_bad_scheme(imviz_helper): From b46c00147b1bf95f1d48a3bdeddc692650005bea Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Mon, 22 Jul 2024 12:49:40 -0400 Subject: [PATCH 26/29] add warning message in tray if PIXAR_SR not present in FITS header --- .../plugins/spectral_extraction/spectral_extraction.py | 2 +- .../specviz/plugins/unit_conversion/unit_conversion.py | 10 ++++++++++ .../plugins/unit_conversion/unit_conversion.vue | 5 +++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py index f4a76f14fb..3822b0fce9 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py @@ -501,7 +501,7 @@ def extract(self, return_bg=False, add_data=True, **kwargs): if 'PIXAR_SR' not in self.spectral_cube.meta: snackbar_message = SnackbarMessage( ("PIXAR_SR FITS header keyword not found when parsing spectral cube. " - "Flux/Surface Brightness will use default scale factor of 1."), + "Flux/Surface Brightness will use default PIXAR_SR value of 1."), color="warning", sender=self) self.hub.broadcast(snackbar_message) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 76d1739c4c..b08609523f 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -71,6 +71,10 @@ class UnitConversion(PluginTemplateMixin): # we need to disable translation in the API and UI variables/functions. flux_or_sb_config_disabler = 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 + pixar_sr_exists = Bool(True).tag(sync=True) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -284,6 +288,12 @@ def _on_flux_unit_changed(self, msg): sender=self)) self.spectrum_viewer.reset_limits() + if ( + len(self.app.data_collection) > 0 and self.app.config == 'cubeviz' + and not self.app.data_collection[0].meta.get('PIXAR_SR') + ): + self.pixar_sr_exists = False + def _translate(self, flux_or_sb=None): # currently unsupported, can be supported with a scale factor if self.app.config == 'specviz': diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index 4f3c9e2e22..5eae0ec7d2 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -63,5 +63,10 @@ Translation is not available due to current unit selection. + + PIXAR_SR FITS header keyword not found when parsing spectral cube. + Flux/Surface Brightness will use default PIXAR_SR value of 1. + + From 6eb01c08098538149ae988be44c2484cde8914b0 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 23 Jul 2024 11:40:07 -0400 Subject: [PATCH 27/29] second pass at technical changes --- jdaviz/app.py | 1 - .../spectral_extraction.py | 2 +- .../unit_conversion/unit_conversion.py | 30 ++++++++----------- jdaviz/core/validunits.py | 1 - 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index de0aacdf72..f1f99dd3e3 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -76,7 +76,6 @@ def equivalent_units(self, data, cid, units): '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)', 'erg / (s cm2 Angstrom)', 'erg / (s cm2 Angstrom)', 'erg / (s cm2 Hz)', 'erg / (Hz s cm2)', 'ph / (Angstrom s cm2)', diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py index 3822b0fce9..33cc3509d7 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py @@ -501,7 +501,7 @@ def extract(self, return_bg=False, add_data=True, **kwargs): if 'PIXAR_SR' not in self.spectral_cube.meta: snackbar_message = SnackbarMessage( ("PIXAR_SR FITS header keyword not found when parsing spectral cube. " - "Flux/Surface Brightness will use default PIXAR_SR value of 1."), + "Flux/Surface Brightness will use default PIXAR_SR value of 1 sr/pix^2."), color="warning", sender=self) self.hub.broadcast(snackbar_message) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index b08609523f..71ff0bec05 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -115,13 +115,14 @@ def __init__(self, *args, **kwargs): @property def user_api(self): if self.app.config == 'cubeviz': - return PluginUserApi(self, expose=('spectral_unit', 'flux_or_sb', 'flux_unit', 'sb_unit')) # noqa - if self.app.config == 'specviz' and not self.flux_or_sb_config_disabler: - return PluginUserApi(self, expose=('spectral_unit', 'flux_unit', 'sb_unit')) - if self.flux_or_sb_config_disabler == 'Flux': - return PluginUserApi(self, expose=('spectral_unit', 'sb_unit')) - if self.flux_or_sb_config_disabler == 'Surface Brightness': - return PluginUserApi(self, expose=('spectral_unit', 'flux_unit')) + expose = ('spectral_unit', 'flux_or_sb', 'flux_unit', 'sb_unit') + elif self.app.config == 'specviz' and not self.flux_or_sb_config_disabler: + expose = ('spectral_unit', 'flux_unit', 'sb_unit') + elif self.flux_or_sb_config_disabler == 'Flux': + expose = ('spectral_unit', 'sb_unit') + else: # self.flux_or_sb_config_disabler == 'Surface Brightness' + expose = ('spectral_unit', 'flux_unit') + return PluginUserApi(self, expose=expose) def _on_glue_x_display_unit_changed(self, x_unit): if x_unit is None: @@ -223,14 +224,7 @@ def _on_flux_unit_changed(self, msg): data_collection_unit = 'Flux' self.flux_or_sb_config_disabler = 'Surface Brightness' - # skip if arg is not a dictionary - if not isinstance(msg, dict): - raise TypeError("Expected msg to be a dictionary") - # skip if 'name' key is missing name = msg.get('name') - if not name: - raise KeyError("Expected 'name' key in msg") - # determine if flux or surface brightness unit was changed by user if name == 'flux_unit_selected': # when the configuration is Specviz, translation is not currently supported. @@ -289,7 +283,7 @@ def _on_flux_unit_changed(self, msg): self.spectrum_viewer.reset_limits() if ( - len(self.app.data_collection) > 0 and self.app.config == 'cubeviz' + len(self.app.data_collection) > 0 and not self.app.data_collection[0].meta.get('PIXAR_SR') ): self.pixar_sr_exists = False @@ -305,10 +299,10 @@ def _translate(self, flux_or_sb=None): untranslatable_units = units_to_strings(untranslatable_units) if hasattr(self, 'flux_unit'): - if (self.flux_unit.selected in untranslatable_units) \ - and (flux_or_sb == 'Surface Brightness'): + if ((self.flux_unit.selected in untranslatable_units) + and (flux_or_sb == 'Surface Brightness')): raise ValueError( - f"Selected flux unit is not translatable. Please choose a flux unit " + "Selected flux unit is not translatable. Please choose a flux unit " f"that is not in the following list: {untranslatable_units}." ) diff --git a/jdaviz/core/validunits.py b/jdaviz/core/validunits.py index f3cc4b0612..8880eb39f1 100644 --- a/jdaviz/core/validunits.py +++ b/jdaviz/core/validunits.py @@ -71,7 +71,6 @@ def create_flux_equivalencies_list(flux_unit, spectral_axis_unit): locally_defined_flux_units = ['Jy', 'mJy', 'uJy', 'MJy', 'W / (Hz m2)', 'eV / (s m2 Hz)', - 'erg / (s cm2)', 'erg / (s cm2 Hz)', 'erg / (s cm2 Angstrom)', 'ph / (Angstrom s cm2)', From 9067ef5ad089b84ba25528a140bbfd83f6c5b35d Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 23 Jul 2024 12:59:09 -0400 Subject: [PATCH 28/29] move flux_unit checks to beginning of _on_flux_unit_changed --- .../specviz/plugins/unit_conversion/unit_conversion.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 71ff0bec05..d666217e7b 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -206,6 +206,11 @@ def _on_spectral_unit_changed(self, *args): @observe('flux_or_sb_selected', 'flux_unit_selected', 'sb_unit_selected') def _on_flux_unit_changed(self, msg): + # may need to be updated if translations in other configs going to be supported + if not hasattr(self, 'flux_unit'): + return + if not self.flux_unit.choices and self.app.config == 'cubeviz': + return flux_or_sb = None current_y = self.spectrum_viewer.state.y_display_unit @@ -255,7 +260,7 @@ def _on_flux_unit_changed(self, msg): # If in Cubeviz, all spectra pass through Spectral Xxtraction plugin and will # have a scale factor assigned in the metadata, enabling translation. raise ValueError( - f"Unit translation between Flux and Surface Brightness " + "Unit translation between Flux and Surface Brightness " f"is not supported in {self.app.config}." ) flux_or_sb = self.sb_unit.selected @@ -271,9 +276,6 @@ def _on_flux_unit_changed(self, msg): return yunit = _valid_glue_display_unit(flux_or_sb, self.spectrum_viewer, 'y') - # may need to be updated if translations in other configs going to be supported - if not self.flux_unit.choices and self.app.config == 'cubeviz': - return if self.spectrum_viewer.state.y_display_unit != yunit: self.spectrum_viewer.state.y_display_unit = yunit From 069dbbcce0c57656cea818287afa29f3faadbfa7 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 23 Jul 2024 13:09:53 -0400 Subject: [PATCH 29/29] change _unit to flux.unit --- .../configs/specviz/plugins/unit_conversion/unit_conversion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index d666217e7b..39655cdf20 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -222,7 +222,7 @@ def _on_flux_unit_changed(self, msg): self.app.data_collection[0] and self.app.config == 'specviz' ): - if check_if_unit_is_per_solid_angle(self.app.data_collection[0].get_object()._unit): + if check_if_unit_is_per_solid_angle(self.app.data_collection[0].get_object().flux.unit): # noqa data_collection_unit = 'Surface Brightness' self.flux_or_sb_config_disabler = 'Flux' else: