From 15c35149906e63516bc321fa930f924e93fe6054 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 26 Dec 2024 12:55:40 -0500 Subject: [PATCH] address Kyle's review comments --- CHANGES.rst | 2 +- .../tests/test_spectral_extraction.py | 6 +++--- jdaviz/configs/default/plugins/markers/markers.py | 6 ------ .../default/plugins/model_fitting/model_fitting.py | 9 +-------- jdaviz/configs/imviz/plugins/coords_info/coords_info.py | 5 ++++- .../specviz/plugins/unit_conversion/unit_conversion.py | 3 +-- jdaviz/core/helpers.py | 9 +++++++-- 7 files changed, 17 insertions(+), 23 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 0cb216b460..63e4ca3c78 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -19,7 +19,7 @@ Specviz Specviz2d ^^^^^^^^^ -- Implement the Unit Conversion plugin the Specviz2D. [#3253] +- Implement the Unit Conversion plugin in Specviz2D. [#3253] API Changes ----------- 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 dda85ffd4e..aac421dd91 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 @@ -524,10 +524,10 @@ def test_spectral_extraction_with_correct_sum_units(cubeviz_helper, assert '_pixel_scale_factor' in collapsed.meta # Original units in Jy / sr - expected_flux_values = [190., 590., 990., 1390., 1790., 2190., 2590., 2990., 3390., 3790.] # After collapsing, sr is removed via the scale factor and the extracted spectrum is in Jy - expected_flux_values = [flux * collapsed.meta.get('_pixel_scale_factor') - for flux in expected_flux_values] + expected_flux_values = (np.array([190., 590., 990., 1390., 1790., + 2190., 2590., 2990., 3390., 3790.]) * + collapsed.meta.get('_pixel_scale_factor')) np.testing.assert_allclose( collapsed.flux.value, diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index 55b8eee68a..960ede6a40 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -225,12 +225,6 @@ def _on_is_active_changed(self, *args): def _on_viewer_key_event(self, viewer, data): if data['event'] == 'keydown' and data['key'] == 'm': row_info = self.coords_info.as_dict() - if 'value' in row_info: - # format and cast flux value for Table - row_info['value'] = f"{row_info['value']:.5e}" - else: - row_info['value'] = '' - if 'viewer' in self.table.headers_avail: row_info['viewer'] = viewer.reference if viewer.reference is not None else viewer.reference_id # noqa diff --git a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py index ee24121b0e..5605d12c62 100644 --- a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py @@ -554,7 +554,6 @@ def _initialize_model_component(self, model_comp, comp_label, poly_order=None): # equivs for spectral density and flux<>sb pixar_sr = masked_spectrum.meta.get('_pixel_scale_factor', 1.0) equivs = all_flux_unit_conversion_equivs(pixar_sr, init_x) - equivs += u.spectral_density(init_x) init_y = flux_conversion_general([init_y.value], init_y.unit, @@ -906,13 +905,7 @@ def _fit_model_to_spectrum(self, add_data): return models_to_fit = self._reinitialize_with_fixed() - spec = self.dataset.selected_spectrum - # Needs Attention: - # should conversion occur before or after call to _apply_subset_masks? - if spec.flux.unit.to_string != self.app._get_display_unit('flux'): - equivalencies = u.spectral_density(self.dataset.selected_spectrum.spectral_axis) - spec = self.dataset.selected_spectrum.with_flux_unit(self.app._get_display_unit('flux'), - equivalencies=equivalencies) + spec = self.dataset.get_selected_spectrum(use_display_units=True) masked_spectrum = self._apply_subset_masks(spec, self.spectral_subset) diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index 42ef83689e..dfcce317cd 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -559,7 +559,10 @@ def _image_viewer_update(self, viewer, x, y): else: dq_text = '' self.row1b_text = f'{value:+10.5e} {unit}{dq_text}' - self._dict['value'] = float(value) + if not isinstance(value, (float, np.floating)): + self._dict['value'] = float(value) + else: + self._dict['value'] = value self._dict['value:unit'] = str(unit) self._dict['value:unreliable'] = unreliable_pixel else: diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 53976de355..ad4288cc36 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -114,8 +114,7 @@ def __init__(self, *args, **kwargs): self._cached_properties = ['image_layers'] if self.config not in ['specviz', 'specviz2d', 'cubeviz']: - # TODO [specviz2d, mosviz] x_display_unit is not implemented in glue for image viewer - # used by spectrum-2d-viewer + # TODO [mosviz] x_display_unit is not implemented in glue for image viewer # TODO [mosviz]: add to yaml file # TODO [cubeviz, slice]: slice indicator broken after changing spectral_unit # TODO: support for multiple viewers and handling of mixed state from glue (or does diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 3a745eae5a..40a33170e9 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -501,7 +501,11 @@ def _handle_display_units(self, data, use_display_units=True): eqv=eqv) new_uncert = StdDevUncertainty(new_uncert_converted, unit=y_unit) else: - new_uncert = StdDevUncertainty(new_uncert, unit=data.flux.unit) + eqv = all_flux_unit_conversion_equivs(cube_wave=data.spectral_axis) + new_uncert_converted = flux_conversion(new_uncert.quantity.value, + new_uncert.unit, y_unit, spec=data, + eqv=eqv) + new_uncert = StdDevUncertainty(new_uncert, unit=y_unit) else: new_uncert = None @@ -512,8 +516,9 @@ def _handle_display_units(self, data, use_display_units=True): new_y = flux_conversion(data.flux.value, data.flux.unit, y_unit, data, eqv=eqv) * u.Unit(y_unit) else: + eqv = all_flux_unit_conversion_equivs(cube_wave=data.spectral_axis) new_y = flux_conversion(data.flux.value, data.flux.unit, - data.flux.unit, spec=data) * u.Unit(data.flux.unit) + y_unit, spec=data) * u.Unit(y_unit) new_spec = (spectral_axis_conversion(data.spectral_axis.value, data.spectral_axis.unit,