From 3f80eb5426a12d25fa892707679ad7c72016c636 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Mon, 9 Sep 2024 10:19:46 -0400 Subject: [PATCH 1/3] per-config display of unit selections (#3166) * per-config display of unit selections and customizable downstream --- .../unit_conversion/unit_conversion.py | 53 ++++++++++++---- .../unit_conversion/unit_conversion.vue | 63 ++++++++++++------- 2 files changed, 80 insertions(+), 36 deletions(-) diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 27d2de23ae..e5212dcaa7 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -56,17 +56,25 @@ class UnitConversion(PluginTemplateMixin): """ template_file = __file__, "unit_conversion.vue" + has_spectral = Bool(False).tag(sync=True) spectral_unit_items = List().tag(sync=True) spectral_unit_selected = Unicode().tag(sync=True) + has_flux = Bool(False).tag(sync=True) flux_unit_items = List().tag(sync=True) flux_unit_selected = Unicode().tag(sync=True) - sb_unit_selected = Unicode().tag(sync=True) - + has_angle = Bool(False).tag(sync=True) angle_unit_items = List().tag(sync=True) angle_unit_selected = Unicode().tag(sync=True) + has_sb = Bool(False).tag(sync=True) + sb_unit_selected = Unicode().tag(sync=True) + + has_time = Bool(False).tag(sync=True) + time_unit_items = List().tag(sync=True) + time_unit_selected = Unicode().tag(sync=True) + spectral_y_type_items = List().tag(sync=True) spectral_y_type_selected = Unicode().tag(sync=True) @@ -97,30 +105,51 @@ def __init__(self, *args, **kwargs): self.session.hub.subscribe(self, AddDataToViewerMessage, handler=self._find_and_convert_contour_units) + self.has_spectral = self.config in ('specviz', 'cubeviz', 'specviz2d', 'mosviz') self.spectral_unit = UnitSelectPluginComponent(self, items='spectral_unit_items', selected='spectral_unit_selected') - self.spectral_y_type = SelectPluginComponent(self, - items='spectral_y_type_items', - selected='spectral_y_type_selected', - manual_options=['Surface Brightness', 'Flux']) - + self.has_flux = self.config in ('specviz', 'cubeviz', 'specviz2d', 'mosviz') self.flux_unit = UnitSelectPluginComponent(self, items='flux_unit_items', selected='flux_unit_selected') + self.has_angle = self.config in ('cubeviz', 'specviz', 'mosviz') self.angle_unit = UnitSelectPluginComponent(self, items='angle_unit_items', selected='angle_unit_selected') + self.has_sb = self.has_angle or self.config in ('imviz',) + # NOTE: always read_only, exposed through sb_unit property + + self.has_time = False + self.time_unit = UnitSelectPluginComponent(self, + items='time_unit_items', + selected='time_unit_selected') + + self.spectral_y_type = SelectPluginComponent(self, + items='spectral_y_type_items', + selected='spectral_y_type_selected', + manual_options=['Surface Brightness', 'Flux']) + @property def user_api(self): - if self.app.config == 'cubeviz': - expose = ('spectral_unit', 'spectral_y_type', 'flux_unit', 'angle_unit', 'sb_unit') - else: - expose = ('spectral_unit', 'flux_unit', 'angle_unit') - return PluginUserApi(self, expose=expose) + expose = [] + readonly = [] + if self.has_spectral: + expose += ['spectral_unit'] + if self.has_flux: + expose += ['flux_unit'] + if self.has_angle: + expose += ['angle_unit'] + if self.has_sb: + readonly = ['sb_unit'] + if self.has_time: + expose += ['time_unit'] + if self.config == 'cubeviz': + expose += ['spectral_y_type'] + return PluginUserApi(self, expose=expose, readonly=readonly) @property def sb_unit(self): diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index 1f8a26d8d7..e04b2b74e3 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -9,7 +9,7 @@ :popout_button="popout_button" :scroll_to.sync="scroll_to"> - + - + + + + + - - + + - + - - - +
+ + + - - - + + + - - PIXAR_SR FITS header keyword not found when parsing spectral cube. - Flux/Surface Brightness will use default PIXAR_SR value of 1. - + + PIXAR_SR FITS header keyword not found when parsing spectral cube. + Flux/Surface Brightness will use default PIXAR_SR value of 1. + +
From 9c1f6a91a984239a53d39b6fd0f5b7a4de57f25a Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> Date: Mon, 9 Sep 2024 10:49:56 -0400 Subject: [PATCH 2/3] Fix empty flux menu in Specviz with spectrum in surface brightness units (#3185) * Debugging * Return from flux_unit_selected if no angle unit * Trying to set spectral y type on first data load * Working fix that doesn't break tests * Changelog * Don't set this if unit conversion doesn't exist * Add test that fails on main * Codestyle * Update jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py --- CHANGES.rst | 2 +- jdaviz/configs/specviz/plugins/parsers.py | 10 ++++++++++ .../unit_conversion/tests/test_unit_conversion.py | 9 +++++++++ .../specviz/plugins/unit_conversion/unit_conversion.py | 10 +++++++++- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1aea7de067..b0660b8951 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, #2940, #3088, #3111, #3113, #3129, #3139, #3149, #3155, #3178] + unit conversion in Cubeviz and Specviz. [#2781, #2940, #3088, #3111, #3113, #3129, #3139, #3149, #3155, #3178, #3185] - Plugin tray is now open by default. [#2892] diff --git a/jdaviz/configs/specviz/plugins/parsers.py b/jdaviz/configs/specviz/plugins/parsers.py index 51fbfc1ee6..0ae7546d95 100644 --- a/jdaviz/configs/specviz/plugins/parsers.py +++ b/jdaviz/configs/specviz/plugins/parsers.py @@ -11,6 +11,7 @@ from jdaviz.core.events import SnackbarMessage from jdaviz.core.registries import data_parser_registry from jdaviz.utils import standardize_metadata, download_uri_to_path +from jdaviz.core.validunits import check_if_unit_is_per_solid_angle __all__ = ["specviz_spectrum1d_parser"] @@ -159,6 +160,15 @@ def specviz_spectrum1d_parser(app, data, data_label=None, format=None, show_in_v # Make metadata layout conform with other viz. spec.meta = standardize_metadata(spec.meta) + # If this is the first loaded data, we want to set spectral y unit type to Flux or + # Surface Brightness as appropriate + if len(app.data_collection) == 0 and "Unit Conversion" in app._jdaviz_helper.plugins: + uc = app._jdaviz_helper.plugins["Unit Conversion"] + if check_if_unit_is_per_solid_angle(flux_units): + uc._obj.spectral_y_type = "Surface Brightness" + else: + uc._obj.spectral_y_type = "Flux" + app.add_data(spec, data_label[i]) # handle display, with the SpectrumList special case in mind. 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 7501bb83ea..f6607d745d 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 @@ -35,6 +35,15 @@ def test_value_error_exception(specviz_helper, spectrum1d, new_spectral_axis, ne assert u.Unit(viewer.state.y_display_unit) == u.Unit(expected_flux) +def test_initialize_specviz_sb(specviz_helper, spectrum1d): + spec_sb = Spectrum1D(spectrum1d.flux/u.sr, spectrum1d.spectral_axis) + specviz_helper.load_data(spec_sb, data_label="Test 1D Spectrum") + plg = specviz_helper.plugins["Unit Conversion"] + assert plg._obj.flux_unit == "Jy" + assert plg._obj.spectral_y_type == "Surface Brightness" + assert plg._obj.angle_unit == "sr" + + @pytest.mark.parametrize('uncert', (False, True)) def test_conv_wave_only(specviz_helper, spectrum1d, uncert): if uncert is False: diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index e5212dcaa7..bca8ff5bde 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -173,7 +173,7 @@ def _on_glue_x_display_unit_changed(self, x_unit_str): self.spectral_unit.selected = x_unit_str if not len(self.flux_unit.choices) or not len(self.angle_unit.choices): # in case flux_unit was triggered first (but could not be set because there - # as no spectral_unit to determine valid equivalencies) + # was no spectral_unit to determine valid equivalencies) self._on_glue_y_display_unit_changed(self.spectrum_viewer.state.y_display_unit) def _on_glue_y_display_unit_changed(self, y_unit_str): @@ -204,6 +204,14 @@ def _on_glue_y_display_unit_changed(self, y_unit_str): if check_if_unit_is_per_solid_angle(y_unit_str): flux_choices = create_flux_equivalencies_list(y_unit * u.sr, x_unit) self.flux_unit.choices = flux_choices + flux_unit = str(y_unit * u.sr) + # We need to set the angle_unit before triggering _on_flux_unit_changed via + # setting self.flux_unit.selected below, or the lack of angle unit will make it think + # we're in Flux units. + self.angle_unit.choices = create_angle_equivalencies_list(y_unit) + self.angle_unit.selected = self.angle_unit.choices[0] + if flux_unit in self.flux_unit.choices and flux_unit != self.flux_unit.selected: + self.flux_unit.selected = flux_unit # sets the angle unit drop down and the surface brightness read-only text if self.app.data_collection[0]: From deaab7922adb8374eb54aacb92db4d388f7668d1 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:09:57 -0400 Subject: [PATCH 3/3] BUG: Fix spectral extraction in the presence of unit conversion (no actual conversion, no new callback) (#3177) * TST: Add a test that fails on main but we want to fix here * Spectral Extraction: Enforce user flux unit when spectrum viewer becomes empty before overwrite happens. TST: Adjust original test because actual values no longer converted. * Unleash patch on all Cubeviz plugins as kecnry requested * Add xfail test for general viewer case * Fix test * Fix PEP 8 --- .../tests/test_spectral_extraction.py | 18 ++++++++++++++++++ jdaviz/configs/specviz/tests/test_viewers.py | 15 +++++++++++++++ jdaviz/core/template_mixin.py | 9 +++++++++ 3 files changed, 42 insertions(+) diff --git a/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py b/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py index 295432051e..0cfc4044d6 100644 --- a/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py +++ b/jdaviz/configs/cubeviz/plugins/spectral_extraction/tests/test_spectral_extraction.py @@ -570,6 +570,24 @@ def test_default_spectral_extraction(cubeviz_helper, spectrum1d_cube_fluxunit_jy ) +def test_spectral_extraction_unit_conv_one_spec( + cubeviz_helper, spectrum1d_cube_fluxunit_jy_per_steradian +): + cubeviz_helper.load_data(spectrum1d_cube_fluxunit_jy_per_steradian) + spectrum_viewer = cubeviz_helper.app.get_viewer( + cubeviz_helper._default_spectrum_viewer_reference_name) + uc = cubeviz_helper.plugins["Unit Conversion"] + assert uc.flux_unit == "Jy" + uc.flux_unit.selected = "MJy" + spec_extr_plugin = cubeviz_helper.plugins['Spectral Extraction'] + # Overwrite the one and only default extraction. + collapsed = spec_extr_plugin.extract() + # Actual values not in display unit but should not affect display unit. + assert collapsed.flux.unit == u.Jy + assert uc.flux_unit.selected == "MJy" + assert spectrum_viewer.state.y_display_unit == "MJy" + + @pytest.mark.usefixtures('_jail') @pytest.mark.remote_data @pytest.mark.parametrize( diff --git a/jdaviz/configs/specviz/tests/test_viewers.py b/jdaviz/configs/specviz/tests/test_viewers.py index 2c40e390ff..ba706c32e1 100644 --- a/jdaviz/configs/specviz/tests/test_viewers.py +++ b/jdaviz/configs/specviz/tests/test_viewers.py @@ -26,6 +26,21 @@ def test_spectrum_viewer_axis_labels(specviz_helper, input_unit, y_axis_label): assert (y_axis_label in label) +@pytest.mark.xfail(reason="FIXME: Some callback magic needs to happen somewhere.") +def test_spectrum_viewer_keep_unit_when_removed(specviz_helper, spectrum1d): + specviz_helper.load_data(spectrum1d, data_label="Test") + uc = specviz_helper.plugins["Unit Conversion"] + assert uc.flux_unit == "Jy" + uc.flux_unit = "MJy" + specviz_helper.app.remove_data_from_viewer("spectrum-viewer", "Test") + specviz_helper.app.add_data_to_viewer("spectrum-viewer", "Test") + # Actual values not in display unit but should not affect display unit. + spec = specviz_helper.get_spectra(data_label="Test", apply_slider_redshift=False) + assert spec.flux.unit == u.Jy + assert uc.flux_unit.selected == "MJy" + assert specviz_helper.app._get_display_unit('spectral_y') == "MJy" + + class TestResetLimitsTwoTests: """See https://github.com/spacetelescope/lcviz/pull/93""" diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index fd7ff237c3..7d1c27be61 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -3910,7 +3910,13 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): add_to_viewer_vis = [True] preserved_attributes = [{}] + enforce_flux_unit = None if label in self.app.data_collection: + if self.app.config == "cubeviz": + sv = self.app.get_viewer( + self.app._jdaviz_helper._default_spectrum_viewer_reference_name) + if len(sv.state.layers) == 1: + enforce_flux_unit = self.app._get_display_unit('spectral_y') for viewer_ref in add_to_viewer_refs: self.app.remove_data_from_viewer(viewer_ref, label) self.app.data_collection.remove(self.app.data_collection[label]) @@ -3943,6 +3949,9 @@ def add_results_from_plugin(self, data_item, replace=None, label=None): label, visible=visible, clear_other_data=this_replace) + if enforce_flux_unit: + sv.state.y_display_unit = enforce_flux_unit + if preserved != {}: layer_state = [layer.state for layer in this_viewer.layers if layer.layer.label == label][0]