From 7152a2da32537b1c46a3224b5392b3e2895b5065 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 1 Oct 2024 14:36:12 -0400 Subject: [PATCH 01/32] first pass specviz2d implementation --- jdaviz/configs/mosviz/mosviz.yaml | 1 + .../specviz/plugins/unit_conversion/unit_conversion.py | 2 +- jdaviz/configs/specviz2d/specviz2d.yaml | 1 + jdaviz/core/marks.py | 4 ++++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/jdaviz/configs/mosviz/mosviz.yaml b/jdaviz/configs/mosviz/mosviz.yaml index ba0344e5ac..b41f7e1c16 100644 --- a/jdaviz/configs/mosviz/mosviz.yaml +++ b/jdaviz/configs/mosviz/mosviz.yaml @@ -20,6 +20,7 @@ tray: - g-plot-options - g-subset-tools - g-markers + - g-unit-conversion - g-gaussian-smooth - g-slit-overlay - g-model-fitting diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index c9b6372885..f00380d520 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -113,7 +113,7 @@ def __init__(self, *args, **kwargs): self._cached_properties = ['image_layers'] - if self.config not in ['specviz', 'cubeviz']: + if self.config not in ['specviz', 'specviz2d', 'cubeviz', 'mosviz']: # TODO [specviz2d, mosviz] x_display_unit is not implemented in glue for image viewer # used by spectrum-2d-viewer # TODO [mosviz]: add to yaml file diff --git a/jdaviz/configs/specviz2d/specviz2d.yaml b/jdaviz/configs/specviz2d/specviz2d.yaml index 056a2d3c51..69c3759aef 100644 --- a/jdaviz/configs/specviz2d/specviz2d.yaml +++ b/jdaviz/configs/specviz2d/specviz2d.yaml @@ -19,6 +19,7 @@ tray: - g-plot-options - g-subset-tools - g-markers + - g-unit-conversion - spectral-extraction - g-gaussian-smooth - g-model-fitting diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 870da67412..a9e6fc90e7 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -110,6 +110,7 @@ def append_xy(self, x, y): self.y = np.append(self.y, y) def set_x_unit(self, unit=None): + print('ya') if unit is None: if not hasattr(self.viewer.state, 'x_display_unit'): return @@ -123,6 +124,9 @@ def set_x_unit(self, unit=None): self.xunit = unit def set_y_unit(self, unit=None): + with open('ex.txt', 'a') as file: + file.write(f'yo\n') + print('yo') if unit is None: if not hasattr(self.viewer.state, 'y_display_unit'): return From 274c917ae0ace68449d6ce0383435af4ae390a23 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 18 Oct 2024 09:42:25 -0400 Subject: [PATCH 02/32] resolve stash --- .../configs/default/plugins/markers/markers.py | 16 ++++++++++++++-- .../plugins/unit_conversion/unit_conversion.py | 18 +++++++++++++----- .../unit_conversion/unit_conversion.vue | 2 +- jdaviz/core/marks.py | 13 +++++-------- jdaviz/core/validunits.py | 0 5 files changed, 33 insertions(+), 16 deletions(-) create mode 100644 jdaviz/core/validunits.py diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index 7379a1c3ff..0cf3bc8b39 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -4,7 +4,7 @@ from jdaviz.core.events import (ViewerAddedMessage, ChangeRefDataMessage, AddDataMessage, RemoveDataMessage, - MarkersPluginUpdate) + MarkersPluginUpdate, GlobalDisplayUnitChanged) from jdaviz.core.marks import MarkersMark from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import PluginTemplateMixin, ViewerSelectMixin, TableMixin @@ -60,10 +60,11 @@ def __init__(self, *args, **kwargs): 'world_ra', 'world_dec', 'world:unreliable', 'value', 'value:unit', 'value:unreliable', 'viewer'] - + elif self.config == 'specviz': headers = ['spectral_axis', 'spectral_axis:unit', 'index', 'value', 'value:unit'] + elif self.config == 'specviz2d': # TODO: add "index" if/when specviz2d supports plotting spectral_axis headers = ['spectral_axis', 'spectral_axis:unit', @@ -114,8 +115,19 @@ def _create_viewer_callbacks(self, viewer): def _on_viewer_added(self, msg): self._create_viewer_callbacks(self.app.get_viewer_by_id(msg.viewer_id)) + + def _on_global_display_unit_changed(self, msg, viewer=None): + print(msg.axis) + #print(msg) + + # all cubes are converted to surface brightness so we just need to + # listen to SB for cubeviz unit changes + if msg.axis == "flux": + self.image_unit = u.Unit(msg.unit) + def _recompute_mark_positions(self, viewer): + print('do we go in here?') if self.table is None or self.table._qtable is None: return if 'world_ra' not in self.table.headers_avail: diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index f00380d520..81888da695 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -39,7 +39,10 @@ def _valid_glue_display_unit(unit_str, sv, axis='x'): def _flux_to_sb_unit(flux_unit, angle_unit): + print('flux_unit = ', flux_unit) + print(angle_unit) if angle_unit not in supported_sq_angle_units(as_strings=True): + print('first if') sb_unit = flux_unit else: # str > unit > str to remove formatting inconsistencies with @@ -141,7 +144,7 @@ def __init__(self, *args, **kwargs): # initialize flux choices to empty list, will be populated when data is loaded self.flux_unit.choices = [] - self.has_angle = self.config in ('cubeviz', 'specviz', 'mosviz') + self.has_angle = self.config in ('cubeviz', 'specviz', 'mosviz','specviz2d') self.angle_unit = UnitSelectPluginComponent(self, items='angle_unit_items', selected='angle_unit_selected') @@ -211,7 +214,7 @@ def _on_add_data_to_viewer(self, msg): or not len(self.flux_unit_selected) or not len(self.angle_unit_selected) or (self.config == 'cubeviz' and not len(self.spectral_y_type_selected))): - + print('heresssss') data_obj = msg.data.get_object() if isinstance(data_obj, Spectrum1D): @@ -237,9 +240,13 @@ def _on_add_data_to_viewer(self, msg): try: if angle_unit is None: - # default to sr if input spectrum is not in surface brightness units - # TODO: for cubeviz, should we check the cube itself? - self.angle_unit.selected = 'sr' + if self.config in ['specviz', 'specviz2d']: + self.has_angle = False + self.has_sb = False + else: + # default to pix2 if input spectrum is not in surface brightness units + # TODO: for cubeviz, should we check the cube itself? + self.angle_unit.selected = 'pix2' else: self.angle_unit.selected = str(angle_unit) except ValueError: @@ -299,6 +306,7 @@ def _on_unit_selected(self, msg): return axis = msg.get('name').split('_')[0] + print('axis', axis) if axis == 'spectral': xunit = _valid_glue_display_unit(self.spectral_unit.selected, self.spectrum_viewer, 'x') diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue index 2e8b6ed89a..0454e11089 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.vue @@ -49,7 +49,7 @@ hint="Solid angle unit." /> - + Date: Fri, 18 Oct 2024 09:48:21 -0400 Subject: [PATCH 03/32] fix issues with stash and update get_data --- jdaviz/configs/cubeviz/helper.py | 3 +++ jdaviz/configs/specviz2d/helper.py | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/jdaviz/configs/cubeviz/helper.py b/jdaviz/configs/cubeviz/helper.py index 4c610506b7..5029efe170 100644 --- a/jdaviz/configs/cubeviz/helper.py +++ b/jdaviz/configs/cubeviz/helper.py @@ -141,6 +141,9 @@ def get_data(self, data_label=None, spatial_subset=None, spectral_subset=None, Spectral subset applied to data. cls : `~specutils.Spectrum1D`, `~astropy.nddata.CCDData`, optional The type that data will be returned as. + use_display_units : bool, optional + Specify whether the returned data is in native units or the current + display units. Returns ------- diff --git a/jdaviz/configs/specviz2d/helper.py b/jdaviz/configs/specviz2d/helper.py index 87522333cb..fde23cf10f 100644 --- a/jdaviz/configs/specviz2d/helper.py +++ b/jdaviz/configs/specviz2d/helper.py @@ -171,7 +171,8 @@ def load_trace(self, trace, data_label, show_in_viewer=True): self._default_spectrum_2d_viewer_reference_name, data_label ) - def get_data(self, data_label=None, spectral_subset=None, cls=None): + def get_data(self, data_label=None, spectral_subset=None, + cls=None, use_display_units=False): """ Returns data with name equal to data_label of type cls with subsets applied from spectral_subset. @@ -184,6 +185,9 @@ def get_data(self, data_label=None, spectral_subset=None, cls=None): Spectral subset applied to data. cls : `~specutils.Spectrum1D`, `~astropy.nddata.CCDData`, optional The type that data will be returned as. + use_display_units : bool, optional + Specify whether the returned data is in native units or the current + display units. Returns ------- @@ -191,4 +195,5 @@ def get_data(self, data_label=None, spectral_subset=None, cls=None): Data is returned as type cls with subsets applied. """ - return self._get_data(data_label=data_label, spectral_subset=spectral_subset, cls=cls) + return self._get_data(data_label=data_label, spectral_subset=spectral_subset, + cls=cls, use_display_units=use_display_units) From f1372139a6930bb05542493aec258d3acac12728 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Mon, 21 Oct 2024 15:06:11 -0400 Subject: [PATCH 04/32] second pass specviz2d uc implementation --- jdaviz/configs/cubeviz/helper.py | 3 +-- jdaviz/configs/default/plugins/markers/markers.py | 9 ++------- jdaviz/configs/mosviz/mosviz.yaml | 1 - .../specviz/plugins/unit_conversion/unit_conversion.py | 9 ++------- jdaviz/configs/specviz2d/helper.py | 3 +-- jdaviz/core/marks.py | 1 + 6 files changed, 7 insertions(+), 19 deletions(-) diff --git a/jdaviz/configs/cubeviz/helper.py b/jdaviz/configs/cubeviz/helper.py index 5029efe170..f9579514f1 100644 --- a/jdaviz/configs/cubeviz/helper.py +++ b/jdaviz/configs/cubeviz/helper.py @@ -142,8 +142,7 @@ def get_data(self, data_label=None, spatial_subset=None, spectral_subset=None, cls : `~specutils.Spectrum1D`, `~astropy.nddata.CCDData`, optional The type that data will be returned as. use_display_units : bool, optional - Specify whether the returned data is in native units or the current - display units. + Specify whether the returned data is in native units or the current display units. Returns ------- diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index 0cf3bc8b39..b41e7d96cc 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -60,7 +60,7 @@ def __init__(self, *args, **kwargs): 'world_ra', 'world_dec', 'world:unreliable', 'value', 'value:unit', 'value:unreliable', 'viewer'] - + elif self.config == 'specviz': headers = ['spectral_axis', 'spectral_axis:unit', 'index', 'value', 'value:unit'] @@ -115,19 +115,14 @@ def _create_viewer_callbacks(self, viewer): def _on_viewer_added(self, msg): self._create_viewer_callbacks(self.app.get_viewer_by_id(msg.viewer_id)) - - def _on_global_display_unit_changed(self, msg, viewer=None): - print(msg.axis) - #print(msg) + def _on_global_display_unit_changed(self, msg, viewer=None): # all cubes are converted to surface brightness so we just need to # listen to SB for cubeviz unit changes if msg.axis == "flux": self.image_unit = u.Unit(msg.unit) - def _recompute_mark_positions(self, viewer): - print('do we go in here?') if self.table is None or self.table._qtable is None: return if 'world_ra' not in self.table.headers_avail: diff --git a/jdaviz/configs/mosviz/mosviz.yaml b/jdaviz/configs/mosviz/mosviz.yaml index b41f7e1c16..ba0344e5ac 100644 --- a/jdaviz/configs/mosviz/mosviz.yaml +++ b/jdaviz/configs/mosviz/mosviz.yaml @@ -20,7 +20,6 @@ tray: - g-plot-options - g-subset-tools - g-markers - - g-unit-conversion - g-gaussian-smooth - g-slit-overlay - g-model-fitting diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 81888da695..72055b8ab0 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -39,10 +39,7 @@ def _valid_glue_display_unit(unit_str, sv, axis='x'): def _flux_to_sb_unit(flux_unit, angle_unit): - print('flux_unit = ', flux_unit) - print(angle_unit) if angle_unit not in supported_sq_angle_units(as_strings=True): - print('first if') sb_unit = flux_unit else: # str > unit > str to remove formatting inconsistencies with @@ -144,7 +141,7 @@ def __init__(self, *args, **kwargs): # initialize flux choices to empty list, will be populated when data is loaded self.flux_unit.choices = [] - self.has_angle = self.config in ('cubeviz', 'specviz', 'mosviz','specviz2d') + self.has_angle = self.config in ('cubeviz', 'specviz', 'mosviz', 'specviz2d') self.angle_unit = UnitSelectPluginComponent(self, items='angle_unit_items', selected='angle_unit_selected') @@ -214,7 +211,6 @@ def _on_add_data_to_viewer(self, msg): or not len(self.flux_unit_selected) or not len(self.angle_unit_selected) or (self.config == 'cubeviz' and not len(self.spectral_y_type_selected))): - print('heresssss') data_obj = msg.data.get_object() if isinstance(data_obj, Spectrum1D): @@ -244,7 +240,7 @@ def _on_add_data_to_viewer(self, msg): self.has_angle = False self.has_sb = False else: - # default to pix2 if input spectrum is not in surface brightness units + # default to pix2 if input data is not in surface brightness units # TODO: for cubeviz, should we check the cube itself? self.angle_unit.selected = 'pix2' else: @@ -306,7 +302,6 @@ def _on_unit_selected(self, msg): return axis = msg.get('name').split('_')[0] - print('axis', axis) if axis == 'spectral': xunit = _valid_glue_display_unit(self.spectral_unit.selected, self.spectrum_viewer, 'x') diff --git a/jdaviz/configs/specviz2d/helper.py b/jdaviz/configs/specviz2d/helper.py index fde23cf10f..0fc5d95f73 100644 --- a/jdaviz/configs/specviz2d/helper.py +++ b/jdaviz/configs/specviz2d/helper.py @@ -186,8 +186,7 @@ def get_data(self, data_label=None, spectral_subset=None, cls : `~specutils.Spectrum1D`, `~astropy.nddata.CCDData`, optional The type that data will be returned as. use_display_units : bool, optional - Specify whether the returned data is in native units or the current - display units. + Specify whether the returned data is in native units or the current display units. Returns ------- diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 4b8ce39abd..d0d04cf45e 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -151,6 +151,7 @@ def _on_global_display_unit_changed(self, msg): return if self.viewer.__class__.__name__ in ['SpecvizProfileView', 'CubevizProfileView', + 'MosvizProfileView', 'MosvizProfile2DView']: axis_map = {'spectral': 'x', 'spectral_y': 'y'} else: From 640cd629f016ad8548e7f00d45bcbe53f9bf1f69 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Mon, 21 Oct 2024 16:07:50 -0400 Subject: [PATCH 05/32] revert 2D viewer for uc --- jdaviz/core/marks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index d0d04cf45e..e053c017cb 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -151,9 +151,10 @@ def _on_global_display_unit_changed(self, msg): return if self.viewer.__class__.__name__ in ['SpecvizProfileView', 'CubevizProfileView', - 'MosvizProfileView', - 'MosvizProfile2DView']: + 'MosvizProfileView']: axis_map = {'spectral': 'x', 'spectral_y': 'y'} + elif self.viewer.__class__.__name__ == 'MosvizProfile2DView': + axis_map = {'spectral': 'x'} else: return axis = axis_map.get(msg.axis, None) From 58516a0cae14a28d5d4f947e70ba627b00b6f386 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Mon, 21 Oct 2024 22:29:05 -0400 Subject: [PATCH 06/32] don't allow conversion of spectral line y values --- jdaviz/core/marks.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index e053c017cb..5251134121 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -123,6 +123,8 @@ def set_x_unit(self, unit=None): self.xunit = unit def set_y_unit(self, unit=None): + if self.__class__.__name__ == 'SpectralLine': + return if unit is None: if not hasattr(self.viewer.state, 'y_display_unit'): return From 124d829c08c0cdb84178c80ce62a471369c1712c Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 23 Oct 2024 23:07:34 -0400 Subject: [PATCH 07/32] fix bug to ensure non-scale factor data converts on _handle_display_units --- jdaviz/core/helpers.py | 4 ++-- jdaviz/core/marks.py | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index f5d935a0c2..b708370de1 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -377,10 +377,10 @@ def _handle_display_units(self, data, use_display_units=True): new_uncert = None if ('_pixel_scale_factor' in data.meta): new_y = flux_conversion(data.flux.value, data.flux.unit, - y_unit, data) * u.Unit(y_unit) + y_unit, spec=data) * u.Unit(y_unit) else: 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(data.flux.unit) new_spec = (spectral_axis_conversion(data.spectral_axis.value, data.spectral_axis.unit, spectral_unit) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 5251134121..6f729c5feb 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -123,8 +123,6 @@ def set_x_unit(self, unit=None): self.xunit = unit def set_y_unit(self, unit=None): - if self.__class__.__name__ == 'SpectralLine': - return if unit is None: if not hasattr(self.viewer.state, 'y_display_unit'): return @@ -161,6 +159,12 @@ def _on_global_display_unit_changed(self, msg): return axis = axis_map.get(msg.axis, None) if axis is not None: + scale = self.scales.get(axis, None) + # if PluginMark mark is LinearScale(0, 1), prevent it from entering unit conversion + # machinery so it maintains it's position in viewer. + if isinstance(scale, LinearScale) and (scale.min, scale.max) == (0, 1): + return + getattr(self, f'set_{axis}_unit')(msg.unit) def clear(self): From a025530d5a8b0755222a1164bb3ed9edbf9e267a Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 23 Oct 2024 23:18:18 -0400 Subject: [PATCH 08/32] fix get_data bug using native unit for non-scale factor data --- jdaviz/core/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index b708370de1..25a95b76b1 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -380,7 +380,7 @@ def _handle_display_units(self, data, use_display_units=True): y_unit, spec=data) * u.Unit(y_unit) else: new_y = flux_conversion(data.flux.value, data.flux.unit, - y_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, spectral_unit) From 35d7733cbaa2f366994b7612cc05c0e1a914e085 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 01:02:38 -0400 Subject: [PATCH 09/32] add line analysis uc test --- .../line_analysis/tests/test_line_analysis.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py index 6b077a14ca..7dd2de01b5 100644 --- a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py +++ b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py @@ -6,6 +6,7 @@ from numpy.testing import assert_allclose from regions import RectanglePixelRegion, PixCoord from specutils import Spectrum1D, SpectralRegion +from glue.core.roi import XRangeROI from jdaviz.configs.specviz.plugins.line_analysis.line_analysis import _coerce_unit from jdaviz.core.custom_units_and_equivs import PIX2 @@ -124,6 +125,26 @@ def test_cubeviz_units(cubeviz_helper, spectrum1d_cube_custom_fluxunit, np.testing.assert_allclose(float(results[0]['uncertainty']), float(line_flux_before_unit_conversion['uncertainty'])) + viewer = cubeviz_helper.app.get_viewer('spectrum-viewer') + viewer.apply_roi(XRangeROI(4.63e-7, 4.64e-7)) + + la = cubeviz_helper.plugins['Line Analysis'] + la.keep_active = True + la.spectral_subset.selected = 'Subset 1' + + marks_before = [la._obj.continuum_marks['left'].y, + la._obj.continuum_marks['right'].y] + + uc.flux_unit.selected = 'Jy' + + # multiply converted continuum marks by expected scale factor (MJy -> Jy) + scaling_factor = 1e-6 + marks_after = [la._obj.continuum_marks['left'].y * scaling_factor, + la._obj.continuum_marks['right'].y * scaling_factor] + + # ensure continuum marks update to when flux unit is converted + np.testing.assert_allclose(marks_before, marks_after, rtol=1e-5) + def test_user_api(specviz_helper, spectrum1d): label = "Test 1D Spectrum" From aad7c6fc164424d0aceeb82959d186e57cbedeb3 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 01:27:45 -0400 Subject: [PATCH 10/32] add test for solid angle equivalency list --- jdaviz/core/tests/test_validunits.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 jdaviz/core/tests/test_validunits.py diff --git a/jdaviz/core/tests/test_validunits.py b/jdaviz/core/tests/test_validunits.py new file mode 100644 index 0000000000..c1674c10a8 --- /dev/null +++ b/jdaviz/core/tests/test_validunits.py @@ -0,0 +1,25 @@ +import astropy.units as u +import pytest + +from jdaviz.core.validunits import check_if_unit_is_per_solid_angle, create_angle_equivalencies_list # noqa +from jdaviz.core.custom_units import PIX2 + + +@pytest.mark.parametrize("unit, is_solid_angle", [ + ('erg / sr', True), ('erg / (deg * deg)', True), ('erg / (deg * arcsec)', True), + ('erg / (deg**2)', True), ('erg deg**-2', True), ('erg sr^-1', True), + ('erg / degree', False), ('erg sr^-2', False)]) +def test_check_if_unit_is_per_solid_angle(unit, is_solid_angle): + # test function that tests if a unit string or u.Unit is per solid angle + assert check_if_unit_is_per_solid_angle(unit) == is_solid_angle + + # turn string into u.Unit, and make sure it also passes + assert check_if_unit_is_per_solid_angle(u.Unit(unit)) == is_solid_angle + + +@pytest.mark.parametrize("solid_angle, expected_solid_angle_list", [ + ('sr', ['sr']), ('pix2', ['pix2']), (PIX2, ['pix2']), (None, ['pix2'])]) +def test_angle_equivalency_list(solid_angle, expected_solid_angle_list): + # test the created angle equivalencies list for various solid angle inputs + assert create_angle_equivalencies_list(solid_angle) == expected_solid_angle_list + From acd369325491a350cb937c00135362dde22b5eb8 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 02:28:46 -0400 Subject: [PATCH 11/32] add MosvizProfileView test --- .../tests/test_unit_conversion.py | 30 +++++++++++++++++++ jdaviz/core/helpers.py | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) 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 9dbc4dd800..61ea7b3cc3 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 @@ -154,3 +154,33 @@ def test_flux_unit_choices(specviz_helper, flux_unit, expected_choices): assert uc_plg.flux_unit.selected == flux_unit.to_string() assert uc_plg.flux_unit.choices == expected_choices + + +def test_mosviz_profile_view_mouseover(specviz2d_helper, spectrum2d): + data = np.zeros((5, 10)) + data[3] = np.arange(10) + spectrum2d = Spectrum1D(flux=data*u.MJy, spectral_axis=data[3]*u.um) + + specviz2d_helper.load_data(spectrum2d) + viewer = specviz2d_helper.app.get_viewer("spectrum-viewer") + plg = specviz2d_helper.plugins["Unit Conversion"] + + label_mouseover = specviz2d_helper.app.session.application._tools['g-coords-info'] + label_mouseover._viewer_mouse_event(viewer, + {'event': 'mousemove', + 'domain': {'x': 5, 'y': 3}}) + + assert label_mouseover.as_text() == ('Cursor 5.00000e+00, 3.00000e+00', + 'Wave 5.00000e+00 um (5 pix)', + 'Flux 5.00000e+00 MJy') + + plg._obj.flux_unit_selected = 'Jy' + assert label_mouseover.as_text() == ('Cursor 5.00000e+00, 3.00000e+00', + 'Wave 5.00000e+00 um (5 pix)', + 'Flux 5.00000e+06 Jy') + + # test mouseover when spectral density equivalencies are required for conversion + plg._obj.flux_unit_selected = 'erg / (Angstrom s cm2)' + assert label_mouseover.as_text() == ('Cursor 5.00000e+00, 3.00000e+00', + 'Wave 5.00000e+00 um (5 pix)', + 'Flux 5.99585e-08 erg / (Angstrom s cm2)') diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 25a95b76b1..fc1bb23a25 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -371,7 +371,7 @@ def _handle_display_units(self, data, use_display_units=True): new_uncert.unit, y_unit, spec=data) new_uncert = StdDevUncertainty(new_uncert_converted, unit=y_unit) else: - new_uncert = StdDevUncertainty(new_uncert, unit=data.flux.unit) + new_uncert = StdDevUncertainty(new_uncert, unit=y_unit) else: new_uncert = None From 72dbf9d3206b483ad3af513c169c4649ba52fac1 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 10:28:40 -0400 Subject: [PATCH 12/32] add change log --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 81d9bf952d..985c0d22a2 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -252,6 +252,8 @@ Specviz Specviz2d ^^^^^^^^^ +- Implement the Unit Conversion plugin the Specviz2D. [#3253] + 4.0 (2024-10-17) ================ From 05ca47611f3f75b1810c1b0fd2b7496d3780f29b Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 16:24:12 -0400 Subject: [PATCH 13/32] reconcile test failures --- CHANGES.rst | 4 ++-- .../specviz/plugins/unit_conversion/unit_conversion.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 985c0d22a2..dc4ad04e3c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -36,6 +36,8 @@ Specviz Specviz2d ^^^^^^^^^ +- Implement the Unit Conversion plugin the Specviz2D. [#3253] + API Changes ----------- @@ -252,8 +254,6 @@ Specviz Specviz2d ^^^^^^^^^ -- Implement the Unit Conversion plugin the Specviz2D. [#3253] - 4.0 (2024-10-17) ================ diff --git a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py index 72055b8ab0..53976de355 100644 --- a/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py +++ b/jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py @@ -113,7 +113,7 @@ def __init__(self, *args, **kwargs): self._cached_properties = ['image_layers'] - if self.config not in ['specviz', 'specviz2d', 'cubeviz', 'mosviz']: + 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]: add to yaml file From e86342b0f862ff811c15e06b2fc88f525bb48b46 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 16:44:39 -0400 Subject: [PATCH 14/32] add API test --- .../plugins/unit_conversion/tests/test_unit_conversion.py | 7 +++++++ jdaviz/core/helpers.py | 4 ---- 2 files changed, 7 insertions(+), 4 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 61ea7b3cc3..971d99b811 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 @@ -165,6 +165,13 @@ def test_mosviz_profile_view_mouseover(specviz2d_helper, spectrum2d): viewer = specviz2d_helper.app.get_viewer("spectrum-viewer") plg = specviz2d_helper.plugins["Unit Conversion"] + # make sure we don't expose angle, sb, nor spectral-y units when native + # units are in flux + assert hasattr(plg, 'flux_unit') + assert not hasattr(plg, 'angle_unit') + assert not hasattr(plg, 'sb_unit') + assert not hasattr(plg, 'spectral_y_type') + label_mouseover = specviz2d_helper.app.session.application._tools['g-coords-info'] label_mouseover._viewer_mouse_event(viewer, {'event': 'mousemove', diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index fc1bb23a25..69abfcaa92 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -375,10 +375,6 @@ def _handle_display_units(self, data, use_display_units=True): else: new_uncert = None - if ('_pixel_scale_factor' in data.meta): - new_y = flux_conversion(data.flux.value, data.flux.unit, - y_unit, spec=data) * u.Unit(y_unit) - else: new_y = flux_conversion(data.flux.value, data.flux.unit, y_unit, spec=data) * u.Unit(y_unit) new_spec = (spectral_axis_conversion(data.spectral_axis.value, From b0ed10b1c628ab3130d213e7430a501e8308a47e Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 16:53:38 -0400 Subject: [PATCH 15/32] remove global display unit change handler, syntax error in helpers --- jdaviz/configs/default/plugins/markers/markers.py | 8 +------- jdaviz/core/helpers.py | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index b41e7d96cc..934adfe038 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -4,7 +4,7 @@ from jdaviz.core.events import (ViewerAddedMessage, ChangeRefDataMessage, AddDataMessage, RemoveDataMessage, - MarkersPluginUpdate, GlobalDisplayUnitChanged) + MarkersPluginUpdate) from jdaviz.core.marks import MarkersMark from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import PluginTemplateMixin, ViewerSelectMixin, TableMixin @@ -116,12 +116,6 @@ def _create_viewer_callbacks(self, viewer): def _on_viewer_added(self, msg): self._create_viewer_callbacks(self.app.get_viewer_by_id(msg.viewer_id)) - def _on_global_display_unit_changed(self, msg, viewer=None): - # all cubes are converted to surface brightness so we just need to - # listen to SB for cubeviz unit changes - if msg.axis == "flux": - self.image_unit = u.Unit(msg.unit) - def _recompute_mark_positions(self, viewer): if self.table is None or self.table._qtable is None: return diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 69abfcaa92..574590bd9c 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -375,8 +375,8 @@ def _handle_display_units(self, data, use_display_units=True): else: new_uncert = None - new_y = flux_conversion(data.flux.value, data.flux.unit, - y_unit, spec=data) * u.Unit(y_unit) + new_y = flux_conversion(data.flux.value, data.flux.unit, + y_unit, spec=data) * u.Unit(y_unit) new_spec = (spectral_axis_conversion(data.spectral_axis.value, data.spectral_axis.unit, spectral_unit) From fdb6a5ddf81b85384a5598dd68fb8ecb7e2f8d8c Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 18:49:54 -0400 Subject: [PATCH 16/32] ensure continuum marks don't double translate, test coverage --- .../tests/test_cubeviz_unit_conversion.py | 22 +++++++++++++++++++ jdaviz/core/marks.py | 6 +++++ 2 files changed, 28 insertions(+) diff --git a/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_unit_conversion.py b/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_unit_conversion.py index 374c459ce1..390fa48911 100644 --- a/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_unit_conversion.py +++ b/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_unit_conversion.py @@ -5,6 +5,7 @@ from numpy.testing import assert_allclose from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion from specutils import Spectrum1D +from glue.core.roi import XRangeROI from jdaviz.core.custom_units_and_equivs import PIX2, SPEC_PHOTON_FLUX_DENSITY_UNITS @@ -150,6 +151,7 @@ def test_flux_unit_choices(cubeviz_helper, flux_unit, expected_choices): def test_unit_translation(cubeviz_helper, angle_unit): # custom cube so PIXAR_SR is in metadata, and Flux units, and in MJy w, wcs_dict = cubeviz_wcs_dict() + wcs_dict['PIXAR_SR'] = 10 flux = np.zeros((30, 20, 3001), dtype=np.float32) flux[5:15, 1:11, :] = 1 cube = Spectrum1D(flux=flux * u.MJy / angle_unit, wcs=w, meta=wcs_dict) @@ -175,9 +177,29 @@ def test_unit_translation(cubeviz_helper, angle_unit): viewer_1d = cubeviz_helper.app.get_viewer( cubeviz_helper._default_spectrum_viewer_reference_name) + la = cubeviz_helper.plugins['Line Analysis'] + la.keep_active = True + viewer_1d.apply_roi(XRangeROI(6e-7, 6.2e-7)) + la.spectral_subset.selected = 'Subset 2' + + marks_before = [la._obj.continuum_marks['left'].y, + la._obj.continuum_marks['right'].y] + # change global y-units from Flux -> Surface Brightness uc_plg._obj.spectral_y_type_selected = 'Surface Brightness' + if angle_unit == PIX2: + # translation does not alter spectral_y values in viewer + pixar_sr = 1 + else: + pixar_sr = wcs_dict['PIXAR_SR'] + + marks_after = [la._obj.continuum_marks['left'].y * pixar_sr, + la._obj.continuum_marks['right'].y * pixar_sr] + + # ensure continuum marks update when spectral_y_type is changed + assert_allclose(marks_before, marks_after, rtol=1e-5) + assert uc_plg._obj.spectral_y_type_selected == 'Surface Brightness' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 6f729c5feb..c75d198234 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -129,6 +129,12 @@ def set_y_unit(self, unit=None): unit = self.viewer.state.y_display_unit unit = u.Unit(unit) + # spectrum y-values in viewer have already been converted, don't convert again + # if a spectral_y_type is changed, just update the unit + if self.yunit is not None and check_if_unit_is_per_solid_angle(self.yunit) != check_if_unit_is_per_solid_angle(unit): # noqa + self.yunit = unit + return + if self.yunit is not None and not np.all([s == 0 for s in self.y.shape]): if self.viewer.default_class is Spectrum1D: From b538d1ed80422af3e266a933f8f491d5553bd5c0 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 24 Oct 2024 20:30:52 -0400 Subject: [PATCH 17/32] move line analysis test --- .../tests/test_cubeviz_unit_conversion.py | 22 ------- .../default/plugins/markers/markers.py | 1 + .../line_analysis/tests/test_line_analysis.py | 59 ++++++++++++------- jdaviz/conftest.py | 2 +- 4 files changed, 40 insertions(+), 44 deletions(-) diff --git a/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_unit_conversion.py b/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_unit_conversion.py index 390fa48911..374c459ce1 100644 --- a/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_unit_conversion.py +++ b/jdaviz/configs/cubeviz/plugins/tests/test_cubeviz_unit_conversion.py @@ -5,7 +5,6 @@ from numpy.testing import assert_allclose from regions import PixCoord, CirclePixelRegion, RectanglePixelRegion from specutils import Spectrum1D -from glue.core.roi import XRangeROI from jdaviz.core.custom_units_and_equivs import PIX2, SPEC_PHOTON_FLUX_DENSITY_UNITS @@ -151,7 +150,6 @@ def test_flux_unit_choices(cubeviz_helper, flux_unit, expected_choices): def test_unit_translation(cubeviz_helper, angle_unit): # custom cube so PIXAR_SR is in metadata, and Flux units, and in MJy w, wcs_dict = cubeviz_wcs_dict() - wcs_dict['PIXAR_SR'] = 10 flux = np.zeros((30, 20, 3001), dtype=np.float32) flux[5:15, 1:11, :] = 1 cube = Spectrum1D(flux=flux * u.MJy / angle_unit, wcs=w, meta=wcs_dict) @@ -177,29 +175,9 @@ def test_unit_translation(cubeviz_helper, angle_unit): viewer_1d = cubeviz_helper.app.get_viewer( cubeviz_helper._default_spectrum_viewer_reference_name) - la = cubeviz_helper.plugins['Line Analysis'] - la.keep_active = True - viewer_1d.apply_roi(XRangeROI(6e-7, 6.2e-7)) - la.spectral_subset.selected = 'Subset 2' - - marks_before = [la._obj.continuum_marks['left'].y, - la._obj.continuum_marks['right'].y] - # change global y-units from Flux -> Surface Brightness uc_plg._obj.spectral_y_type_selected = 'Surface Brightness' - if angle_unit == PIX2: - # translation does not alter spectral_y values in viewer - pixar_sr = 1 - else: - pixar_sr = wcs_dict['PIXAR_SR'] - - marks_after = [la._obj.continuum_marks['left'].y * pixar_sr, - la._obj.continuum_marks['right'].y * pixar_sr] - - # ensure continuum marks update when spectral_y_type is changed - assert_allclose(marks_before, marks_after, rtol=1e-5) - assert uc_plg._obj.spectral_y_type_selected == 'Surface Brightness' y_display_unit = u.Unit(viewer_1d.state.y_display_unit) diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index 934adfe038..3d8901211f 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -69,6 +69,7 @@ def __init__(self, *args, **kwargs): # TODO: add "index" if/when specviz2d supports plotting spectral_axis headers = ['spectral_axis', 'spectral_axis:unit', 'pixel_x', 'pixel_y', 'value', 'value:unit', 'viewer'] + elif self.config == 'mosviz': headers = ['spectral_axis', 'spectral_axis:unit', 'pixel_x', 'pixel_y', 'world_ra', 'world_dec', 'index', diff --git a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py index 7dd2de01b5..7c055378d0 100644 --- a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py +++ b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py @@ -92,7 +92,7 @@ def test_cubeviz_units(cubeviz_helper, spectrum1d_cube_custom_fluxunit, is in flux/pix2 and flux/sr, and that the results remain consistant between translations of the spectral y axis flux<>surface brightness. """ - cube = spectrum1d_cube_custom_fluxunit(fluxunit=u.Jy / sq_angle_unit, + cube = spectrum1d_cube_custom_fluxunit(fluxunit=u.MJy / sq_angle_unit, shape=(25, 25, 4), with_uncerts=True) cubeviz_helper.load_data(cube, data_label="Test Cube") @@ -108,9 +108,46 @@ def test_cubeviz_units(cubeviz_helper, spectrum1d_cube_custom_fluxunit, results = plugin.results assert results[0]['unit'] == 'W / m2' + viewer = cubeviz_helper.app.get_viewer('spectrum-viewer') + viewer.apply_roi(XRangeROI(4.63e-7, 4.64e-7)) + + la = cubeviz_helper.plugins['Line Analysis'] + la.keep_active = True + la.spectral_subset.selected = 'Subset 1' + + marks_before = [la._obj.continuum_marks['left'].y, + la._obj.continuum_marks['right'].y] + + # change flux unit and make sure result stays the same after conversion + uc.flux_unit.selected = 'Jy' + + marks_after = [la._obj.continuum_marks['left'].y, + la._obj.continuum_marks['right'].y] + + # ensure continuum marks update when spectral_y is changed by + # multiply converted continuum marks by expected scale factor (MJy -> Jy) + scaling_factor = 1e6 + assert_allclose([mark * scaling_factor for mark in marks_before], marks_after, rtol=1e-5) + + # reset to test again after spectral_y_type is changed + marks_before = marks_after + # now change to surface brightness uc.spectral_y_type = 'Surface Brightness' + if sq_angle_unit == PIX2: + # translation does not alter spectral_y values in viewer + scaling_factor = 1 + else: + scaling_factor = cube.meta.get('PIXAR_SR') + + marks_after = [la._obj.continuum_marks['left'].y, + la._obj.continuum_marks['right'].y] + + # ensure continuum marks update when spectral_y_type is changed + # multiply converted continuum marks by expected pixel scale factor + assert_allclose([mark / scaling_factor for mark in marks_before], marks_after, rtol=1e-5) + results = plugin.results line_flux_before_unit_conversion = results[0] # convert back and forth between unit<>str for string format consistency @@ -125,26 +162,6 @@ def test_cubeviz_units(cubeviz_helper, spectrum1d_cube_custom_fluxunit, np.testing.assert_allclose(float(results[0]['uncertainty']), float(line_flux_before_unit_conversion['uncertainty'])) - viewer = cubeviz_helper.app.get_viewer('spectrum-viewer') - viewer.apply_roi(XRangeROI(4.63e-7, 4.64e-7)) - - la = cubeviz_helper.plugins['Line Analysis'] - la.keep_active = True - la.spectral_subset.selected = 'Subset 1' - - marks_before = [la._obj.continuum_marks['left'].y, - la._obj.continuum_marks['right'].y] - - uc.flux_unit.selected = 'Jy' - - # multiply converted continuum marks by expected scale factor (MJy -> Jy) - scaling_factor = 1e-6 - marks_after = [la._obj.continuum_marks['left'].y * scaling_factor, - la._obj.continuum_marks['right'].y * scaling_factor] - - # ensure continuum marks update to when flux unit is converted - np.testing.assert_allclose(marks_before, marks_after, rtol=1e-5) - def test_user_api(specviz_helper, spectrum1d): label = "Test 1D Spectrum" diff --git a/jdaviz/conftest.py b/jdaviz/conftest.py index 748a519905..6c6f7005f7 100644 --- a/jdaviz/conftest.py +++ b/jdaviz/conftest.py @@ -258,7 +258,7 @@ def _create_spectrum1d_cube_with_fluxunit(fluxunit=u.Jy, shape=(2, 2, 4), with_u wcs_dict = {"CTYPE1": "RA---TAN", "CTYPE2": "DEC--TAN", "CTYPE3": "WAVE-LOG", "CRVAL1": 205, "CRVAL2": 27, "CRVAL3": 4.622e-7, "CDELT1": -0.0001, "CDELT2": 0.0001, "CDELT3": 8e-11, - "CRPIX1": 0, "CRPIX2": 0, "CRPIX3": 0, + "CRPIX1": 0, "CRPIX2": 0, "CRPIX3": 0, "PIXAR_SR": 10, # Need these for aperture photometry test. "TELESCOP": "JWST", "BUNIT": fluxunit.to_string(), "PIXAR_A2": 0.01} w = WCS(wcs_dict) From 556b66fa4249b3ef4bcda7b046815c8b8a7184ff Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 29 Nov 2024 15:30:45 -0500 Subject: [PATCH 18/32] remove files that weren't removed during rebase --- jdaviz/core/tests/test_validunits.py | 25 ------------------------- jdaviz/core/validunits.py | 0 2 files changed, 25 deletions(-) delete mode 100644 jdaviz/core/tests/test_validunits.py delete mode 100644 jdaviz/core/validunits.py diff --git a/jdaviz/core/tests/test_validunits.py b/jdaviz/core/tests/test_validunits.py deleted file mode 100644 index c1674c10a8..0000000000 --- a/jdaviz/core/tests/test_validunits.py +++ /dev/null @@ -1,25 +0,0 @@ -import astropy.units as u -import pytest - -from jdaviz.core.validunits import check_if_unit_is_per_solid_angle, create_angle_equivalencies_list # noqa -from jdaviz.core.custom_units import PIX2 - - -@pytest.mark.parametrize("unit, is_solid_angle", [ - ('erg / sr', True), ('erg / (deg * deg)', True), ('erg / (deg * arcsec)', True), - ('erg / (deg**2)', True), ('erg deg**-2', True), ('erg sr^-1', True), - ('erg / degree', False), ('erg sr^-2', False)]) -def test_check_if_unit_is_per_solid_angle(unit, is_solid_angle): - # test function that tests if a unit string or u.Unit is per solid angle - assert check_if_unit_is_per_solid_angle(unit) == is_solid_angle - - # turn string into u.Unit, and make sure it also passes - assert check_if_unit_is_per_solid_angle(u.Unit(unit)) == is_solid_angle - - -@pytest.mark.parametrize("solid_angle, expected_solid_angle_list", [ - ('sr', ['sr']), ('pix2', ['pix2']), (PIX2, ['pix2']), (None, ['pix2'])]) -def test_angle_equivalency_list(solid_angle, expected_solid_angle_list): - # test the created angle equivalencies list for various solid angle inputs - assert create_angle_equivalencies_list(solid_angle) == expected_solid_angle_list - diff --git a/jdaviz/core/validunits.py b/jdaviz/core/validunits.py deleted file mode 100644 index e69de29bb2..0000000000 From 07589206fab3804f2772c366f0f7ca711548096a Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 29 Nov 2024 15:32:00 -0500 Subject: [PATCH 19/32] fix styling --- jdaviz/core/marks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index c75d198234..80bc70880c 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -151,7 +151,6 @@ def set_y_unit(self, unit=None): self.yunit = unit - def _on_global_display_unit_changed(self, msg): if not self.auto_update_units: return From bb956ccc5688a8955a60138d721f8814b1380ca5 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 11 Dec 2024 17:42:25 -0500 Subject: [PATCH 20/32] add missing import --- jdaviz/core/marks.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 80bc70880c..69f2bf1b8f 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -11,7 +11,8 @@ SpectralMarksChangedMessage, RedshiftMessage) from jdaviz.core.unit_conversion_utils import (all_flux_unit_conversion_equivs, - flux_conversion_general) + flux_conversion_general, + check_if_unit_is_per_solid_angle) __all__ = ['OffscreenLinesMarks', 'BaseSpectrumVerticalLine', 'SpectralLine', From e08cf1fa07e88f53b918cd3586893f7f2e58089f Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 17 Dec 2024 14:27:21 -0500 Subject: [PATCH 21/32] add spectral unit support, add eqvs to model fit, coords, marks --- .../plugins/model_fitting/model_fitting.py | 6 +++- .../imviz/plugins/coords_info/coords_info.py | 30 +++++++++++++++++-- jdaviz/core/helpers.py | 22 +++++++++++--- jdaviz/core/marks.py | 5 ++-- 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py index 40313afbde..8fa153e074 100644 --- a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py @@ -1113,8 +1113,12 @@ def _fit_model_to_spectrum(self, add_data): return models_to_fit = self._reinitialize_with_fixed() - masked_spectrum = self._apply_subset_masks(self.dataset.selected_spectrum, + masked_spectrum = self._apply_subset_masks(self.dataset.selected_spectrum(), self.spectral_subset) + if masked_spectrum.flux.unit.to_string != self.app._get_display_unit('flux'): + equivalencies = u.spectral_density(masked_spectrum.spectral_axis) + masked_spectrum = masked_spectrum.with_flux_unit(self.app._get_display_unit('flux'), + equivalencies=equivalencies) try: fitted_model, fitted_spectrum = fit_model_to_spectrum( masked_spectrum, diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index bec01c238c..792a7b9921 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -22,6 +22,7 @@ from jdaviz.core.unit_conversion_utils import (all_flux_unit_conversion_equivs, check_if_unit_is_per_solid_angle, flux_conversion_general) +from jdaviz.utils import flux_conversion __all__ = ['CoordsInfo'] @@ -402,8 +403,21 @@ def _image_viewer_update(self, viewer, x, y): coords_status = False elif isinstance(viewer, MosvizProfile2DView): - self._dict['spectral_axis'] = self._dict['axes_x'] - self._dict['spectral_axis:unit'] = self._dict['axes_x:unit'] + if self.app._get_display_unit('spectral') != self._dict['axes_x:unit']: + self._dict['spectral_axis:unit'] = self.app._get_display_unit('spectral') + try: + wave, pixel = image.coords.pixel_to_world(x, y) + if (wave is not None and + wave.unit.to_string() != self.app._get_display_unit('spectral')): + equivalencies = all_flux_unit_conversion_equivs(cube_wave=wave) + wave = wave.to(self.app._get_display_unit('spectral'), + equivalencies=equivalencies) + self._dict['spectral_axis'] = wave.value + except Exception: # WCS might not be valid # pragma: no cover + coords_status = False + else: + self._dict['spectral_axis:unit'] = self._dict['axes_x:unit'] + self._dict['spectral_axis'] = self._dict['axes_x'] self._dict['value'] = self._dict['axes_y'] self._dict['value:unit'] = self._dict['axes_y:unit'] coords_status = False @@ -433,6 +447,11 @@ def _image_viewer_update(self, viewer, x, y): # use WCS to expose the wavelength for a 2d spectrum shown in pixel space try: wave, pixel = image.coords.pixel_to_world(x, y) + if (wave is not None and + wave.unit.to_string() != self.app._get_display_unit('spectral')): + equivalencies = all_flux_unit_conversion_equivs(cube_wave=wave) + wave = wave.to(self.app._get_display_unit('spectral'), + equivalencies=equivalencies) except Exception: # WCS might not be valid # pragma: no cover coords_status = False else: @@ -489,6 +508,13 @@ def _image_viewer_update(self, viewer, x, y): dq_data = associated_dq_layer.layer.get_data(dq_attribute) dq_value = dq_data[int(round(y)), int(round(x))] unit = u.Unit(image.get_component(attribute).units) + + if unit != self.app._get_display_unit(attribute): + equivalencies = all_flux_unit_conversion_equivs(cube_wave=wave) + value = flux_conversion(value, unit, self.app._get_display_unit(attribute), + eqv=equivalencies) + unit = self.app._get_display_unit(attribute) + elif isinstance(viewer, (CubevizImageView, RampvizImageView)): arr = image.get_component(attribute).data unit = u.Unit(image.get_component(attribute).units) diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 574590bd9c..19309bb9c3 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -28,6 +28,7 @@ from jdaviz.core.events import SnackbarMessage, ExitBatchLoadMessage, SliceSelectSliceMessage from jdaviz.core.template_mixin import show_widget from jdaviz.utils import data_has_valid_wcs, flux_conversion, spectral_axis_conversion +from jdaviz.core.unit_conversion_utils import all_flux_unit_conversion_equivs __all__ = ['ConfigHelper', 'ImageConfigHelper', 'CubeConfigHelper'] @@ -366,17 +367,30 @@ def _handle_display_units(self, data, use_display_units=True): else: # if not specified as NDUncertainty, assume stddev: new_uncert = uncertainty + if ('_pixel_scale_factor' in data.meta): + pixar_sr = data.meta.get('_pixel_scale_factor', None) + eqv = all_flux_unit_conversion_equivs(pixar_sr=pixar_sr, + cube_wave=data.spectral_axis) new_uncert_converted = flux_conversion(new_uncert.quantity.value, - new_uncert.unit, y_unit, spec=data) + new_uncert.unit, y_unit, spec=data, + eqv=eqv) new_uncert = StdDevUncertainty(new_uncert_converted, unit=y_unit) else: - new_uncert = StdDevUncertainty(new_uncert, unit=y_unit) + new_uncert = StdDevUncertainty(new_uncert, unit=data.flux.unit) else: new_uncert = None - new_y = flux_conversion(data.flux.value, data.flux.unit, - y_unit, spec=data) * u.Unit(y_unit) + if ('_pixel_scale_factor' in data.meta): + pixar_sr = data.meta.get('_pixel_scale_factor', None) + eqv = all_flux_unit_conversion_equivs(pixar_sr=pixar_sr, + cube_wave=data.spectral_axis) + new_y = flux_conversion(data.flux.value, data.flux.unit, + y_unit, data, eqv=eqv) * u.Unit(y_unit) + else: + new_y = flux_conversion(data.flux.value, data.flux.unit, + data.flux.unit, spec=data) * u.Unit(data.flux.unit) + new_spec = (spectral_axis_conversion(data.spectral_axis.value, data.spectral_axis.unit, spectral_unit) diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index 69f2bf1b8f..ce2694c5a0 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -157,10 +157,9 @@ def _on_global_display_unit_changed(self, msg): return if self.viewer.__class__.__name__ in ['SpecvizProfileView', 'CubevizProfileView', - 'MosvizProfileView']: + 'MosvizProfileView', + 'MosvizProfile2DView']: axis_map = {'spectral': 'x', 'spectral_y': 'y'} - elif self.viewer.__class__.__name__ == 'MosvizProfile2DView': - axis_map = {'spectral': 'x'} else: return axis = axis_map.get(msg.axis, None) From 0a3a17133870a4800f11c40ccfe4ce7588b02e15 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Tue, 17 Dec 2024 14:40:44 -0500 Subject: [PATCH 22/32] fix syntax error --- jdaviz/configs/default/plugins/model_fitting/model_fitting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py index 8fa153e074..b4dff401e1 100644 --- a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py @@ -1113,7 +1113,7 @@ def _fit_model_to_spectrum(self, add_data): return models_to_fit = self._reinitialize_with_fixed() - masked_spectrum = self._apply_subset_masks(self.dataset.selected_spectrum(), + masked_spectrum = self._apply_subset_masks(self.dataset.selected_spectrum, self.spectral_subset) if masked_spectrum.flux.unit.to_string != self.app._get_display_unit('flux'): equivalencies = u.spectral_density(masked_spectrum.spectral_axis) From c838bd5515903a296eb1b44920f3521b55ed07c4 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 18 Dec 2024 16:18:45 -0500 Subject: [PATCH 23/32] 2dviewer spectral conversion, first iteration of test coverage --- .../default/plugins/markers/markers.py | 2 ++ .../markers/tests/test_markers_plugin.py | 22 +++++++++++++++ .../plugins/model_fitting/model_fitting.py | 16 +++++++---- .../model_fitting/tests/test_fitting.py | 18 ++++++++++++ .../imviz/plugins/coords_info/coords_info.py | 28 ++++++------------- .../configs/mosviz/tests/test_data_loading.py | 5 +++- jdaviz/core/marks.py | 7 ++--- 7 files changed, 69 insertions(+), 29 deletions(-) diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index 3d8901211f..b19a308ed0 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -225,6 +225,8 @@ 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() + # format and cast flux value for Table + row_info['value'] = f"{row_info['value']:.5e}" 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/markers/tests/test_markers_plugin.py b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py index f98644bd44..0c356fd598 100644 --- a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py +++ b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py @@ -4,6 +4,7 @@ import numpy as np from numpy.testing import assert_allclose import pytest +from specutils import Spectrum1D from jdaviz.core.custom_units_and_equivs import PIX2, SPEC_PHOTON_FLUX_DENSITY_UNITS from jdaviz.core.marks import MarkersMark @@ -242,6 +243,27 @@ def test_markers_cubeviz_flux_unit_conversion(cubeviz_helper, assert last_row['value:unit'] == new_cube_unit_str +def test_markers_specviz2d_flux_unit_conversion(specviz2d_helper, spectrum2d): + data = np.zeros((5, 10)) + data[3] = np.arange(10) + spectrum2d = Spectrum1D(flux=data*u.MJy, spectral_axis=data[3, 2]*u.AA) + specviz2d_helper.load_data(spectrum2d) + + uc = specviz2d_helper.plugins['Unit Conversion'] + uc.open_in_tray() + mp = specviz2d_helper.plugins['Markers'] + mp.keep_active = True + + label_mouseover = specviz2d_helper.app.session.application._tools["g-coords-info"] + viewer2d = specviz2d_helper.app.get_viewer("spectrum2d-viewer") + label_mouseover._viewer_mouse_event(viewer2d, {"event": "mousemove", + "domain": {"x": 6, "y": 3}}) + mp._obj._on_viewer_key_event(viewer2d, {'event': 'keydown', + 'key': 'm'}) + + assert label_mouseover.as_dict()['value:unit'] == uc.flux_unit + + class TestImvizMultiLayer(BaseImviz_WCS_NoWCS): def test_markers_layer_cycle(self): label_mouseover = self.imviz.app.session.application._tools['g-coords-info'] diff --git a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py index b4dff401e1..41e99b67d9 100644 --- a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py @@ -559,6 +559,7 @@ 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, @@ -1113,12 +1114,17 @@ def _fit_model_to_spectrum(self, add_data): return models_to_fit = self._reinitialize_with_fixed() - masked_spectrum = self._apply_subset_masks(self.dataset.selected_spectrum, + 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) + + masked_spectrum = self._apply_subset_masks(spec, self.spectral_subset) - if masked_spectrum.flux.unit.to_string != self.app._get_display_unit('flux'): - equivalencies = u.spectral_density(masked_spectrum.spectral_axis) - masked_spectrum = masked_spectrum.with_flux_unit(self.app._get_display_unit('flux'), - equivalencies=equivalencies) + try: fitted_model, fitted_spectrum = fit_model_to_spectrum( masked_spectrum, diff --git a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py index 2ad1baf4db..7f2047d473 100644 --- a/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/tests/test_fitting.py @@ -312,6 +312,8 @@ def test_results_table(specviz_helper, spectrum1d): mf = specviz_helper.plugins['Model Fitting'] mf.create_model_component('Linear1D') + uc = specviz_helper.plugins['Unit Conversion'] + mf.add_results.label = 'linear model' with warnings.catch_warnings(): warnings.filterwarnings('ignore', message='Model is linear in parameters.*') @@ -325,6 +327,9 @@ def test_results_table(specviz_helper, spectrum1d): 'L:intercept_0', 'L:intercept_0:unit', 'L:intercept_0:fixed', 'L:intercept_0:std'] + # verify units in table match the current display unit + assert mf_table['L:intercept_0:unit'][-1] == uc.flux_unit + mf.create_model_component('Gaussian1D') mf.add_results.label = 'composite model' with warnings.catch_warnings(): @@ -346,6 +351,19 @@ def test_results_table(specviz_helper, spectrum1d): 'G:stddev_1', 'G:stddev_1:unit', 'G:stddev_1:fixed', 'G:stddev_1:std'] + mf.remove_model_component('G') + assert len(mf_table) == 2 + + # verify Spectrum1D model fitting plugin and table can handle spectral density conversions + uc.flux_unit = 'erg / (Angstrom s cm2)' + mf.reestimate_model_parameters() + + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', message='Model is linear in parameters.*') + mf.calculate_fit(add_data=True) + + assert mf_table['L:intercept_0:unit'][-1] == uc.flux_unit + def test_equation_validation(specviz_helper, spectrum1d): data_label = 'test' diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index 792a7b9921..42ef83689e 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -403,21 +403,8 @@ def _image_viewer_update(self, viewer, x, y): coords_status = False elif isinstance(viewer, MosvizProfile2DView): - if self.app._get_display_unit('spectral') != self._dict['axes_x:unit']: - self._dict['spectral_axis:unit'] = self.app._get_display_unit('spectral') - try: - wave, pixel = image.coords.pixel_to_world(x, y) - if (wave is not None and - wave.unit.to_string() != self.app._get_display_unit('spectral')): - equivalencies = all_flux_unit_conversion_equivs(cube_wave=wave) - wave = wave.to(self.app._get_display_unit('spectral'), - equivalencies=equivalencies) - self._dict['spectral_axis'] = wave.value - except Exception: # WCS might not be valid # pragma: no cover - coords_status = False - else: - self._dict['spectral_axis:unit'] = self._dict['axes_x:unit'] - self._dict['spectral_axis'] = self._dict['axes_x'] + self._dict['spectral_axis'] = self._dict['axes_x'] + self._dict['spectral_axis:unit'] = self._dict['axes_x:unit'] self._dict['value'] = self._dict['axes_y'] self._dict['value:unit'] = self._dict['axes_y:unit'] coords_status = False @@ -447,11 +434,12 @@ def _image_viewer_update(self, viewer, x, y): # use WCS to expose the wavelength for a 2d spectrum shown in pixel space try: wave, pixel = image.coords.pixel_to_world(x, y) - if (wave is not None and - wave.unit.to_string() != self.app._get_display_unit('spectral')): + if wave is not None: equivalencies = all_flux_unit_conversion_equivs(cube_wave=wave) wave = wave.to(self.app._get_display_unit('spectral'), equivalencies=equivalencies) + self._dict['spectral_axis'] = wave.value + self._dict['spectral_axis:unit'] = wave.unit.to_string() except Exception: # WCS might not be valid # pragma: no cover coords_status = False else: @@ -502,14 +490,16 @@ def _image_viewer_update(self, viewer, x, y): if isinstance(viewer, (ImvizImageView, MosvizImageView, MosvizProfile2DView)): value = image.get_data(attribute)[int(round(y)), int(round(x))] + if associated_dq_layers is not None: associated_dq_layer = associated_dq_layers[0] dq_attribute = associated_dq_layer.state.attribute dq_data = associated_dq_layer.layer.get_data(dq_attribute) dq_value = dq_data[int(round(y)), int(round(x))] - unit = u.Unit(image.get_component(attribute).units) - if unit != self.app._get_display_unit(attribute): + unit = u.Unit(image.get_component(attribute).units) + if (isinstance(viewer, MosvizProfile2DView) and unit != '' + and unit != self.app._get_display_unit(attribute)): equivalencies = all_flux_unit_conversion_equivs(cube_wave=wave) value = flux_conversion(value, unit, self.app._get_display_unit(attribute), eqv=equivalencies) diff --git a/jdaviz/configs/mosviz/tests/test_data_loading.py b/jdaviz/configs/mosviz/tests/test_data_loading.py index 21b0865bc9..ff44722e3c 100644 --- a/jdaviz/configs/mosviz/tests/test_data_loading.py +++ b/jdaviz/configs/mosviz/tests/test_data_loading.py @@ -222,8 +222,11 @@ def test_load_single_image_multi_spec(mosviz_helper, mos_image, spectrum1d, mos_ label_mouseover._viewer_mouse_event(spec2d_viewer, {'event': 'mousemove', 'domain': {'x': 10, 'y': 100}}) + + # Note: spectra2d Wave loaded in meters, but we respect one spectral unit, so the meters in + # converted to Angstrom (the spectra1d spectral unit). assert label_mouseover.as_text() == ('Pixel x=00010.0 y=00100.0 Value +8.12986e-01', - 'Wave 1.10000e-05 m', '') + 'Wave 1.10000e+05 Angstrom', '') assert label_mouseover.icon == 'c' # need to trigger a mouseleave or mouseover to reset the traitlets diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index ce2694c5a0..ef0cd4a497 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -11,8 +11,8 @@ SpectralMarksChangedMessage, RedshiftMessage) from jdaviz.core.unit_conversion_utils import (all_flux_unit_conversion_equivs, - flux_conversion_general, check_if_unit_is_per_solid_angle) +from jdaviz.utils import flux_conversion __all__ = ['OffscreenLinesMarks', 'BaseSpectrumVerticalLine', 'SpectralLine', @@ -136,7 +136,7 @@ def set_y_unit(self, unit=None): self.yunit = unit return - if self.yunit is not None and not np.all([s == 0 for s in self.y.shape]): + if self.yunit is not None and not np.all([s == 0 for s in self.y.shape]): # noqa if self.viewer.default_class is Spectrum1D: @@ -145,8 +145,7 @@ def set_y_unit(self, unit=None): pixar_sr = spec.meta.get('PIXAR_SR', 1) cube_wave = self.x * self.xunit equivs = all_flux_unit_conversion_equivs(pixar_sr, cube_wave) - y = flux_conversion_general(self.y, self.yunit, unit, equivs, - with_unit=False) + y = flux_conversion(self.y, self.yunit, unit, eqv=equivs) self.y = y From c11a3c3a602c87b4baea474306ca54f290042669 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Wed, 18 Dec 2024 22:47:21 -0500 Subject: [PATCH 24/32] resolve test failures --- .../tests/test_spectral_extraction.py | 11 ++++++++++- jdaviz/configs/default/plugins/markers/markers.py | 7 +++++-- .../plugins/markers/tests/test_markers_plugin.py | 4 ++-- jdaviz/core/marks.py | 2 ++ 4 files changed, 19 insertions(+), 5 deletions(-) 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 9893fd5420..dda85ffd4e 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 @@ -520,9 +520,18 @@ def test_spectral_extraction_with_correct_sum_units(cubeviz_helper, cubeviz_helper.load_data(spectrum1d_cube_fluxunit_jy_per_steradian) spec_extr_plugin = cubeviz_helper.plugins['Spectral Extraction']._obj collapsed = spec_extr_plugin.extract() + + 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] + np.testing.assert_allclose( collapsed.flux.value, - [190., 590., 990., 1390., 1790., 2190., 2590., 2990., 3390., 3790.] + expected_flux_values ) assert collapsed.flux.unit == u.Jy assert collapsed.uncertainty.unit == u.Jy diff --git a/jdaviz/configs/default/plugins/markers/markers.py b/jdaviz/configs/default/plugins/markers/markers.py index b19a308ed0..55b8eee68a 100644 --- a/jdaviz/configs/default/plugins/markers/markers.py +++ b/jdaviz/configs/default/plugins/markers/markers.py @@ -225,8 +225,11 @@ 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() - # format and cast flux value for Table - row_info['value'] = f"{row_info['value']:.5e}" + 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/markers/tests/test_markers_plugin.py b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py index 0c356fd598..221ff56c9a 100644 --- a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py +++ b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py @@ -246,7 +246,7 @@ def test_markers_cubeviz_flux_unit_conversion(cubeviz_helper, def test_markers_specviz2d_flux_unit_conversion(specviz2d_helper, spectrum2d): data = np.zeros((5, 10)) data[3] = np.arange(10) - spectrum2d = Spectrum1D(flux=data*u.MJy, spectral_axis=data[3, 2]*u.AA) + spectrum2d = Spectrum1D(flux=data*u.MJy, spectral_axis=data[3]*u.AA) specviz2d_helper.load_data(spectrum2d) uc = specviz2d_helper.plugins['Unit Conversion'] @@ -255,7 +255,7 @@ def test_markers_specviz2d_flux_unit_conversion(specviz2d_helper, spectrum2d): mp.keep_active = True label_mouseover = specviz2d_helper.app.session.application._tools["g-coords-info"] - viewer2d = specviz2d_helper.app.get_viewer("spectrum2d-viewer") + viewer2d = specviz2d_helper.app.get_viewer("spectrum-2d-viewer") label_mouseover._viewer_mouse_event(viewer2d, {"event": "mousemove", "domain": {"x": 6, "y": 3}}) mp._obj._on_viewer_key_event(viewer2d, {'event': 'keydown', diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index ef0cd4a497..ad8d01c1bb 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -141,6 +141,8 @@ def set_y_unit(self, unit=None): if self.viewer.default_class is Spectrum1D: spec = self.viewer.state.reference_data.get_object(cls=Spectrum1D) + if self.xunit is None: + self.xunit = self.viewer.jdaviz_helper.app._get_display_unit('spectral') pixar_sr = spec.meta.get('PIXAR_SR', 1) cube_wave = self.x * self.xunit From 62998b57be6b6a008cf32a4ada32f03419bb264e Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 19 Dec 2024 10:05:42 -0500 Subject: [PATCH 25/32] address spec density edge case, test coverage --- .../markers/tests/test_markers_plugin.py | 38 ++++++++++++++++++- jdaviz/core/marks.py | 5 +-- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py index 221ff56c9a..b096931473 100644 --- a/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py +++ b/jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py @@ -243,7 +243,7 @@ def test_markers_cubeviz_flux_unit_conversion(cubeviz_helper, assert last_row['value:unit'] == new_cube_unit_str -def test_markers_specviz2d_flux_unit_conversion(specviz2d_helper, spectrum2d): +def test_markers_specviz2d_unit_conversion(specviz2d_helper, spectrum2d): data = np.zeros((5, 10)) data[3] = np.arange(10) spectrum2d = Spectrum1D(flux=data*u.MJy, spectral_axis=data[3]*u.AA) @@ -258,10 +258,44 @@ def test_markers_specviz2d_flux_unit_conversion(specviz2d_helper, spectrum2d): viewer2d = specviz2d_helper.app.get_viewer("spectrum-2d-viewer") label_mouseover._viewer_mouse_event(viewer2d, {"event": "mousemove", "domain": {"x": 6, "y": 3}}) + assert label_mouseover.as_text() == ('Pixel x=06.0 y=03.0 Value +6.00000e+00 MJy', + 'Wave 6.00000e+00 Angstrom', + '') mp._obj._on_viewer_key_event(viewer2d, {'event': 'keydown', 'key': 'm'}) - assert label_mouseover.as_dict()['value:unit'] == uc.flux_unit + # make sure last marker added to table reflects new unit selection + last_row = mp.export_table()[-1] + assert last_row['value:unit'] == uc.flux_unit + assert last_row['spectral_axis:unit'] == uc.spectral_unit + + # ensure marks work with flux conversion where spectral axis is required and + # spectral axis conversion + uc.flux_unit = 'erg / (Angstrom s cm2)' + uc.spectral_unit = 'Ry' + label_mouseover._viewer_mouse_event(viewer2d, {"event": "mousemove", + "domain": {"x": 4, "y": 3}}) + assert label_mouseover.as_text() == ('Pixel x=04.0 y=03.0 Value +7.49481e+00 erg / (Angstrom s cm2)', # noqa + 'Wave 2.27817e+02 Ry', + '') + mp._obj._on_viewer_key_event(viewer2d, {'event': 'keydown', + 'key': 'm'}) + + # make sure last marker added to table reflects new unit selection + last_row = mp.export_table()[-1] + assert last_row['value:unit'] == uc.flux_unit + assert last_row['spectral_axis:unit'] == uc.spectral_unit + + second_marker_flux_unit = uc.flux_unit.selected + second_marker_spectral_unit = uc.spectral_unit.selected + + # test edge case two non-native spectral axis required conversions + uc.flux_unit = 'ph / (Angstrom s cm2)' + uc.spectral_unit = 'eV' + + # make sure table flux and spectral unit doesn't update + assert last_row['value:unit'] == second_marker_flux_unit + assert last_row['spectral_axis:unit'] == second_marker_spectral_unit class TestImvizMultiLayer(BaseImviz_WCS_NoWCS): diff --git a/jdaviz/core/marks.py b/jdaviz/core/marks.py index ad8d01c1bb..65c51d781f 100644 --- a/jdaviz/core/marks.py +++ b/jdaviz/core/marks.py @@ -139,10 +139,9 @@ def set_y_unit(self, unit=None): if self.yunit is not None and not np.all([s == 0 for s in self.y.shape]): # noqa if self.viewer.default_class is Spectrum1D: - - spec = self.viewer.state.reference_data.get_object(cls=Spectrum1D) if self.xunit is None: - self.xunit = self.viewer.jdaviz_helper.app._get_display_unit('spectral') + return + spec = self.viewer.state.reference_data.get_object(cls=Spectrum1D) pixar_sr = spec.meta.get('PIXAR_SR', 1) cube_wave = self.x * self.xunit From f1702240ba2d05317958931aad66d18f5d0f8d14 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 26 Dec 2024 12:55:40 -0500 Subject: [PATCH 26/32] 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 dc4ad04e3c..1b83432bdf 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -36,7 +36,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 41e99b67d9..8c0b9f0540 100644 --- a/jdaviz/configs/default/plugins/model_fitting/model_fitting.py +++ b/jdaviz/configs/default/plugins/model_fitting/model_fitting.py @@ -559,7 +559,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, @@ -1114,13 +1113,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 19309bb9c3..52896f1273 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -377,7 +377,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 @@ -388,8 +392,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, From b481be1dc6c3a880e9b608dfd3790c4046930391 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Mon, 30 Dec 2024 15:47:33 -0500 Subject: [PATCH 27/32] resolve test failures, update handle_display_unit for get_selected_spectrum --- jdaviz/conftest.py | 2 +- jdaviz/core/freezable_state.py | 7 ++++++- jdaviz/core/helpers.py | 19 ++++++++++++------- jdaviz/utils.py | 2 +- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/jdaviz/conftest.py b/jdaviz/conftest.py index 6c6f7005f7..ded57b6337 100644 --- a/jdaviz/conftest.py +++ b/jdaviz/conftest.py @@ -258,7 +258,7 @@ def _create_spectrum1d_cube_with_fluxunit(fluxunit=u.Jy, shape=(2, 2, 4), with_u wcs_dict = {"CTYPE1": "RA---TAN", "CTYPE2": "DEC--TAN", "CTYPE3": "WAVE-LOG", "CRVAL1": 205, "CRVAL2": 27, "CRVAL3": 4.622e-7, "CDELT1": -0.0001, "CDELT2": 0.0001, "CDELT3": 8e-11, - "CRPIX1": 0, "CRPIX2": 0, "CRPIX3": 0, "PIXAR_SR": 10, + "CRPIX1": 0, "CRPIX2": 0, "CRPIX3": 0, "PIXAR_SR": 10., # Need these for aperture photometry test. "TELESCOP": "JWST", "BUNIT": fluxunit.to_string(), "PIXAR_A2": 0.01} w = WCS(wcs_dict) diff --git a/jdaviz/core/freezable_state.py b/jdaviz/core/freezable_state.py index fa22fd0523..087aa8ae31 100644 --- a/jdaviz/core/freezable_state.py +++ b/jdaviz/core/freezable_state.py @@ -8,6 +8,7 @@ from glue.viewers.matplotlib.state import DeferredDrawCallbackProperty as DDCProperty from jdaviz.utils import get_reference_image_data, flux_conversion +from jdaviz.core.unit_conversion_utils import all_flux_unit_conversion_equivs __all__ = ['FreezableState', 'FreezableProfileViewerState', 'FreezableBqplotImageViewerState'] @@ -71,15 +72,19 @@ def _convert_units_y_limits(self, old_unit, new_unit): # NOTE: this uses the scale factor from the first found layer. We may want to # generalize this to iterate over all scale factors if/when we support multiple # flux cubes (with potentially different pixel scale factors). + eqv = None for layer in self.layers: if psc := getattr(layer.layer, 'meta', {}).get('_pixel_scale_factor', None): # noqa spectral_axis.info.meta = {'_pixel_scale_factor', psc} + eqv = all_flux_unit_conversion_equivs(pixar_sr=psc, + cube_wave=spectral_axis) break else: spectral_axis.info.meta = {} + eqv = all_flux_unit_conversion_equivs(cube_wave=spectral_axis) - y_corners_new = flux_conversion(y_corners, old_unit, new_unit, spectral_axis=spectral_axis) # noqa + y_corners_new = flux_conversion(y_corners, old_unit, new_unit, spectral_axis=spectral_axis, eqv=eqv) # noqa with delay_callback(self, 'y_min', 'y_max'): self.y_min = np.nanmin(y_corners_new) diff --git a/jdaviz/core/helpers.py b/jdaviz/core/helpers.py index 52896f1273..609a6886bd 100644 --- a/jdaviz/core/helpers.py +++ b/jdaviz/core/helpers.py @@ -28,7 +28,8 @@ from jdaviz.core.events import SnackbarMessage, ExitBatchLoadMessage, SliceSelectSliceMessage from jdaviz.core.template_mixin import show_widget from jdaviz.utils import data_has_valid_wcs, flux_conversion, spectral_axis_conversion -from jdaviz.core.unit_conversion_utils import all_flux_unit_conversion_equivs +from jdaviz.core.unit_conversion_utils import (all_flux_unit_conversion_equivs, + check_if_unit_is_per_solid_angle) __all__ = ['ConfigHelper', 'ImageConfigHelper', 'CubeConfigHelper'] @@ -367,7 +368,6 @@ def _handle_display_units(self, data, use_display_units=True): else: # if not specified as NDUncertainty, assume stddev: new_uncert = uncertainty - if ('_pixel_scale_factor' in data.meta): pixar_sr = data.meta.get('_pixel_scale_factor', None) eqv = all_flux_unit_conversion_equivs(pixar_sr=pixar_sr, @@ -376,13 +376,15 @@ def _handle_display_units(self, data, use_display_units=True): new_uncert.unit, y_unit, spec=data, eqv=eqv) new_uncert = StdDevUncertainty(new_uncert_converted, unit=y_unit) + elif ((check_if_unit_is_per_solid_angle(u.Unit(data.flux.unit)) != + check_if_unit_is_per_solid_angle(u.Unit(y_unit)))): + new_uncert = StdDevUncertainty(new_uncert, unit=data.flux.unit) else: 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) - + new_uncert.unit, y_unit, + spec=data, eqv=eqv) + new_uncert = StdDevUncertainty(new_uncert_converted, unit=y_unit) else: new_uncert = None if ('_pixel_scale_factor' in data.meta): @@ -391,10 +393,13 @@ def _handle_display_units(self, data, use_display_units=True): cube_wave=data.spectral_axis) new_y = flux_conversion(data.flux.value, data.flux.unit, y_unit, data, eqv=eqv) * u.Unit(y_unit) + elif (check_if_unit_is_per_solid_angle(u.Unit(data.flux.unit)) != + check_if_unit_is_per_solid_angle(u.Unit(y_unit))): + new_y = data.flux.value * u.Unit(data.flux.unit) else: eqv = all_flux_unit_conversion_equivs(cube_wave=data.spectral_axis) new_y = flux_conversion(data.flux.value, data.flux.unit, - y_unit, spec=data) * u.Unit(y_unit) + y_unit, spec=data, eqv=eqv) * u.Unit(y_unit) new_spec = (spectral_axis_conversion(data.spectral_axis.value, data.spectral_axis.unit, diff --git a/jdaviz/utils.py b/jdaviz/utils.py index 15b8c6098e..a543282391 100644 --- a/jdaviz/utils.py +++ b/jdaviz/utils.py @@ -519,7 +519,7 @@ def _indirect_conversion(values, orig_units, targ_units, eqv, return values, targ_units, 'targ' elif indirect_needs_spec_axis or (spec_unit and solid_angle_in_spec): - if not solid_angle_in_targ: + if not solid_angle_in_targ and solid_angle_in_spec: targ_units /= solid_angle_in_spec solid_angle_in_targ = solid_angle_in_spec if ((u.Unit(targ_units) in indirect_units()) or From 320d8c85f42c7cdc0e61b45a6d42412ba19decc9 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 3 Jan 2025 14:21:21 -0500 Subject: [PATCH 28/32] ensure spectral axis only passed in correct cases --- jdaviz/core/freezable_state.py | 1 + 1 file changed, 1 insertion(+) diff --git a/jdaviz/core/freezable_state.py b/jdaviz/core/freezable_state.py index 087aa8ae31..1fd824f356 100644 --- a/jdaviz/core/freezable_state.py +++ b/jdaviz/core/freezable_state.py @@ -83,6 +83,7 @@ def _convert_units_y_limits(self, old_unit, new_unit): else: spectral_axis.info.meta = {} eqv = all_flux_unit_conversion_equivs(cube_wave=spectral_axis) + spectral_axis = None y_corners_new = flux_conversion(y_corners, old_unit, new_unit, spectral_axis=spectral_axis, eqv=eqv) # noqa From 9e031c914e30e5d76bcb3dabd9890eb9e5786ce3 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 23 Jan 2025 10:30:03 -0500 Subject: [PATCH 29/32] add uc support for spec extract, support for matched zoom and uc --- jdaviz/configs/mosviz/plugins/tools.py | 34 ++++++++++++++ .../spectral_extraction.py | 45 ++++++++++++------- .../tests/test_spectral_extraction.py | 32 +++++++++++++ jdaviz/conftest.py | 2 +- jdaviz/core/tests/test_tools.py | 29 ++++++++++++ 5 files changed, 125 insertions(+), 17 deletions(-) diff --git a/jdaviz/configs/mosviz/plugins/tools.py b/jdaviz/configs/mosviz/plugins/tools.py index bde5a25f82..c95e3a8479 100644 --- a/jdaviz/configs/mosviz/plugins/tools.py +++ b/jdaviz/configs/mosviz/plugins/tools.py @@ -1,6 +1,7 @@ import os from glue.config import viewer_tool +from astropy import units as u from jdaviz.configs.mosviz.plugins.viewers import MosvizProfileView, MosvizProfile2DView from jdaviz.core.tools import _MatchedZoomMixin, HomeZoom, BoxZoom, XRangeZoom, PanZoom, PanZoomX @@ -18,12 +19,45 @@ def _is_matched_viewer(self, viewer): return isinstance(viewer, (MosvizProfile2DView, MosvizProfileView)) def _map_limits(self, from_viewer, to_viewer, limits={}): + components = self.viewer.state.data_collection[0]._components + # Determine cid for spectral axis + cid = None + for key in components.keys(): + if 'Wavelength' in str(key): + cid = str(key) + break + elif 'Wave' in str(key): + cid = str(key) + break + + if cid is None: + raise ValueError("Neither 'Wavelength' nor 'Wave' component'" + + "found in the data collection.") + + native_unit = u.Unit(self.viewer.state.data_collection[0].get_component(cid).units) + current_display_unit = u.Unit(self.viewer.jdaviz_helper.app._get_display_unit('spectral')) + if isinstance(from_viewer, MosvizProfileView) and isinstance(to_viewer, MosvizProfile2DView): # noqa + if native_unit != current_display_unit: + limits['x_min'] = (limits['x_min'] * native_unit).to_value( + current_display_unit, equivalencies=u.spectral() + ) + + limits['x_max'] = (limits['x_max'] * native_unit).to_value( + current_display_unit, equivalencies=u.spectral() + ) limits['x_min'], limits['x_max'] = to_viewer.world_to_pixel_limits((limits['x_min'], limits['x_max'])) elif isinstance(from_viewer, MosvizProfile2DView) and isinstance(to_viewer, MosvizProfileView): # noqa limits['x_min'], limits['x_max'] = from_viewer.pixel_to_world_limits((limits['x_min'], limits['x_max'])) + if native_unit != current_display_unit: + limits['x_min'] = (limits['x_min'] * native_unit).to_value( + current_display_unit, equivalencies=u.spectral() + ) + limits['x_max'] = (limits['x_max'] * native_unit).to_value( + current_display_unit, equivalencies=u.spectral() + ) return limits diff --git a/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py index 05cad4e168..6e0cfb7ab2 100644 --- a/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py @@ -2,7 +2,6 @@ from traitlets import Bool, List, Unicode, observe - from jdaviz.core.events import SnackbarMessage from jdaviz.core.registries import tray_registry from jdaviz.core.template_mixin import (PluginTemplateMixin, @@ -389,9 +388,10 @@ def _trace_dataset_selected(self, msg=None): # happens when first initializing plugin outside of tray return - width = self.trace_dataset.selected_obj.shape[0] + width = self.trace_dataset.get_selected_spectrum(use_display_units=True).shape[0] # estimate the pixel number by taking the median of the brightest pixel index in each column - brightest_pixel = int(np.median(np.argmax(self.trace_dataset.selected_obj.flux, axis=0))) + trace_flux = self.trace_dataset.get_selected_spectrum(use_display_units=True).flux + brightest_pixel = int(np.median(np.argmax(trace_flux, axis=0))) # do not allow to be an edge pixel if brightest_pixel < 1: brightest_pixel = 1 @@ -708,6 +708,7 @@ def import_trace(self, trace): else: # pragma: no cover raise NotImplementedError(f"trace of type {trace.__class__.__name__} not supported") + # UPDATE HERE @with_spinner('trace_spinner') def export_trace(self, add_data=False, **kwargs): """ @@ -728,21 +729,29 @@ def export_trace(self, add_data=False, **kwargs): # then we're offsetting an existing trace # for FlatTrace, we can keep and expose a new FlatTrace (which has the advantage of # being able to load back into the plugin) - orig_trace = self.trace_trace.selected_obj + orig_trace = self.trace_trace.get_selected_spectrum( + self.trace_trace.selected_obj, use_display_units=True + ) if isinstance(orig_trace, tracing.FlatTrace): - trace = tracing.FlatTrace(self.trace_dataset.selected_obj, + trace = tracing.FlatTrace(self.trace_dataset.get_selected_spectrum( + self.trace_dataset, use_display_units=True), orig_trace.trace_pos+self.trace_offset) else: - trace = tracing.ArrayTrace(self.trace_dataset.selected_obj, - self.trace_trace.selected_obj.trace+self.trace_offset) + trace = tracing.ArrayTrace(self.trace_dataset.get_selected_spectrum( + self.trace_dataset, use_display_units=True), + self.trace_trace.get_selected_spectrum( + self.trace_trace.selected_obj, + use_display_units=True).trace+self.trace_offset) elif self.trace_type_selected == 'Flat': - trace = tracing.FlatTrace(self.trace_dataset.selected_obj, + trace = tracing.FlatTrace(self.trace_dataset.get_selected_spectrum( + use_display_units=True), self.trace_pixel) elif self.trace_type_selected in _model_cls: trace_model = _model_cls[self.trace_type_selected](degree=self.trace_order) - trace = tracing.FitTrace(self.trace_dataset.selected_obj, + trace = tracing.FitTrace(self.trace_dataset.get_selected_spectrum( + use_display_units=True), guess=self.trace_pixel, bins=int(self.trace_bins) if self.trace_do_binning else None, window=self.trace_window, @@ -762,12 +771,13 @@ def vue_create_trace(self, *args): def _get_bg_trace(self): if self.bg_type_selected == 'Manual': - trace = tracing.FlatTrace(self.trace_dataset.selected_obj, + trace = tracing.FlatTrace(self.trace_dataset.get_selected_spectrum( + use_display_units=True), self.bg_trace_pixel) elif self.bg_trace_selected == 'From Plugin': trace = self.export_trace(add_data=False) else: - trace = self.bg_trace.selected_obj + trace = self.bg_trace.get_selected_spectrum(use_disaply_units=True) return trace @@ -825,17 +835,20 @@ def export_bg(self, **kwargs): trace = self._get_bg_trace() if self.bg_type_selected == 'Manual': - bg = background.Background(self.bg_dataset.selected_obj, + bg = background.Background(self.bg_dataset.get_selected_spectrum( + use_display_units=True), [trace], width=self.bg_width, statistic=self.bg_statistic.selected.lower()) elif self.bg_type_selected == 'OneSided': - bg = background.Background.one_sided(self.bg_dataset.selected_obj, + bg = background.Background.one_sided(self.bg_dataset.get_selected_spectrum( + use_display_units=True), trace, self.bg_separation, width=self.bg_width, statistic=self.bg_statistic.selected.lower()) elif self.bg_type_selected == 'TwoSided': - bg = background.Background.two_sided(self.bg_dataset.selected_obj, + bg = background.Background.two_sided(self.bg_dataset.get_selected_spectrum( + use_display_units=True), trace, self.bg_separation, width=self.bg_width, @@ -918,13 +931,13 @@ def _get_ext_trace(self): if self.ext_trace_selected == 'From Plugin': return self.export_trace(add_data=False) else: - return self.ext_trace.selected_obj + return self.ext_trace.get_selected_spectrum(use_display_units=True) def _get_ext_input_spectrum(self): if self.ext_dataset_selected == 'From Plugin': return self.export_bg_sub(add_data=False) else: - return self.ext_dataset.selected_obj + return self.ext_dataset.get_selected_spectrum(use_display_units=True) def import_extract(self, ext): """ diff --git a/jdaviz/configs/specviz2d/plugins/spectral_extraction/tests/test_spectral_extraction.py b/jdaviz/configs/specviz2d/plugins/spectral_extraction/tests/test_spectral_extraction.py index 5f4cecd774..c04515fdfe 100644 --- a/jdaviz/configs/specviz2d/plugins/spectral_extraction/tests/test_spectral_extraction.py +++ b/jdaviz/configs/specviz2d/plugins/spectral_extraction/tests/test_spectral_extraction.py @@ -10,6 +10,8 @@ from specreduce import tracing, background, extract from specutils import Spectrum1D +from jdaviz.core.custom_units_and_equivs import SPEC_PHOTON_FLUX_DENSITY_UNITS + GWCS_LT_0_18_1 = Version(gwcs.__version__) < Version('0.18.1') @@ -265,3 +267,33 @@ def test_horne_extract_self_profile(specviz2d_helper): pext.self_prof_interp_degree_y = 0 with pytest.raises(ValueError, match='`self_prof_interp_degree_y` must be greater than 0.'): sp_ext = pext.export_extract_spectrum() + + +def test_spectral_extraction_flux_unit_conversions(specviz2d_helper, mos_spectrum2d): + specviz2d_helper.load_data(mos_spectrum2d) + + uc = specviz2d_helper.plugins["Unit Conversion"] + pext = specviz2d_helper.plugins['Spectral Extraction'] + + for new_flux_unit in SPEC_PHOTON_FLUX_DENSITY_UNITS: + # iterate through flux units verifying that selected object/spectrum is obtained using + # display units + uc.flux_unit.selected = new_flux_unit + + exported_trace = pext.export_trace() + assert exported_trace.image._unit == specviz2d_helper.app._get_display_unit('flux') + + exported_bg = pext.export_bg() + assert exported_bg.image._unit == specviz2d_helper.app._get_display_unit('flux') + + exported_bg_img = pext.export_bg_img() + assert exported_bg_img._unit == specviz2d_helper.app._get_display_unit('flux') + + exported_bg_sub = pext.export_bg_sub() + assert exported_bg_sub._unit == specviz2d_helper.app._get_display_unit('flux') + + exported_extract_spectrum = pext.export_extract_spectrum() + assert exported_extract_spectrum._unit == specviz2d_helper.app._get_display_unit('flux') + + exported_extract = pext.export_extract() + assert exported_extract.image._unit == specviz2d_helper.app._get_display_unit('flux') diff --git a/jdaviz/conftest.py b/jdaviz/conftest.py index ded57b6337..26d4f37465 100644 --- a/jdaviz/conftest.py +++ b/jdaviz/conftest.py @@ -360,7 +360,7 @@ def _generate_mos_spectrum2d(): 'CRVAL1': 0.0, 'CRVAL2': 5.0, 'RADESYS': 'ICRS', 'SPECSYS': 'BARYCENT'} np.random.seed(42) - data = np.random.sample((1024, 15)) * u.one + data = np.random.sample((1024, 15)) * u.Jy return data, header diff --git a/jdaviz/core/tests/test_tools.py b/jdaviz/core/tests/test_tools.py index 35622814b4..beea25e853 100644 --- a/jdaviz/core/tests/test_tools.py +++ b/jdaviz/core/tests/test_tools.py @@ -141,3 +141,32 @@ def test_stretch_bounds_click_outside_threshold(imviz_helper): stretch_tool.on_mouse_event(outside_threshold_msg) assert po.stretch_vmin.value == initial_vmin assert po.stretch_vmax.value == initial_vmax + + +def test_unit_conversion_limits_update(specviz2d_helper, mos_spectrum2d): + specviz2d_helper.load_data(mos_spectrum2d) + uc = specviz2d_helper.plugins['Unit Conversion'] + + spec_viewer = specviz2d_helper.app.get_viewer( + specviz2d_helper.app._jdaviz_helper._default_spectrum_viewer_reference_name) + spec2d_viewer = specviz2d_helper.app.get_viewer( + specviz2d_helper.app._jdaviz_helper._default_spectrum_2d_viewer_reference_name) + + # ensure spectrum and spectrum2d viewer limits matching updates when spectral_unit + # conversion occurs + uc.spectral_unit = 'Hz' + + spec_viewer_lims_before = spec_viewer.get_limits() + spec2d_viewer_lims_before = spec2d_viewer.get_limits() + + spec_viewer.reset_limits() + + # ensure spectral unit conversion occurs when limits are manually changed + assert_allclose(spec_viewer_lims_before, spec_viewer.get_limits()) + assert_allclose(spec2d_viewer_lims_before, spec2d_viewer.get_limits()) + + spec2d_viewer.reset_limits() + + # test again when matching viewer's limits are reset + assert_allclose(spec_viewer_lims_before, spec_viewer.get_limits()) + assert_allclose(spec2d_viewer_lims_before, spec2d_viewer.get_limits()) From 338979e6c9198aaa1be3e3a125bcceb5dc9e1817 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Thu, 23 Jan 2025 14:38:47 -0500 Subject: [PATCH 30/32] reconcile test failures --- .../imviz/plugins/coords_info/coords_info.py | 2 ++ jdaviz/configs/mosviz/plugins/tools.py | 14 ++++++-------- jdaviz/configs/mosviz/tests/test_data_loading.py | 2 +- jdaviz/configs/specviz2d/tests/test_parsers.py | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py index dfcce317cd..849febcc93 100644 --- a/jdaviz/configs/imviz/plugins/coords_info/coords_info.py +++ b/jdaviz/configs/imviz/plugins/coords_info/coords_info.py @@ -499,6 +499,8 @@ def _image_viewer_update(self, viewer, x, y): unit = u.Unit(image.get_component(attribute).units) if (isinstance(viewer, MosvizProfile2DView) and unit != '' + and u.Unit(self.app._get_display_unit(attribute)).physical_type + not in ['frequency', 'wavelength', 'length'] and unit != self.app._get_display_unit(attribute)): equivalencies = all_flux_unit_conversion_equivs(cube_wave=wave) value = flux_conversion(value, unit, self.app._get_display_unit(attribute), diff --git a/jdaviz/configs/mosviz/plugins/tools.py b/jdaviz/configs/mosviz/plugins/tools.py index c95e3a8479..a99ab5098c 100644 --- a/jdaviz/configs/mosviz/plugins/tools.py +++ b/jdaviz/configs/mosviz/plugins/tools.py @@ -30,19 +30,17 @@ def _map_limits(self, from_viewer, to_viewer, limits={}): cid = str(key) break - if cid is None: - raise ValueError("Neither 'Wavelength' nor 'Wave' component'" + - "found in the data collection.") - - native_unit = u.Unit(self.viewer.state.data_collection[0].get_component(cid).units) + if cid is not None: + native_unit = u.Unit(self.viewer.state.data_collection[0].get_component(cid).units) + else: + native_unit = '' current_display_unit = u.Unit(self.viewer.jdaviz_helper.app._get_display_unit('spectral')) if isinstance(from_viewer, MosvizProfileView) and isinstance(to_viewer, MosvizProfile2DView): # noqa - if native_unit != current_display_unit: + if native_unit != current_display_unit and native_unit != '': limits['x_min'] = (limits['x_min'] * native_unit).to_value( current_display_unit, equivalencies=u.spectral() ) - limits['x_max'] = (limits['x_max'] * native_unit).to_value( current_display_unit, equivalencies=u.spectral() ) @@ -51,7 +49,7 @@ def _map_limits(self, from_viewer, to_viewer, limits={}): elif isinstance(from_viewer, MosvizProfile2DView) and isinstance(to_viewer, MosvizProfileView): # noqa limits['x_min'], limits['x_max'] = from_viewer.pixel_to_world_limits((limits['x_min'], limits['x_max'])) - if native_unit != current_display_unit: + if native_unit != current_display_unit and native_unit != '': limits['x_min'] = (limits['x_min'] * native_unit).to_value( current_display_unit, equivalencies=u.spectral() ) diff --git a/jdaviz/configs/mosviz/tests/test_data_loading.py b/jdaviz/configs/mosviz/tests/test_data_loading.py index ff44722e3c..398494e729 100644 --- a/jdaviz/configs/mosviz/tests/test_data_loading.py +++ b/jdaviz/configs/mosviz/tests/test_data_loading.py @@ -225,7 +225,7 @@ def test_load_single_image_multi_spec(mosviz_helper, mos_image, spectrum1d, mos_ # Note: spectra2d Wave loaded in meters, but we respect one spectral unit, so the meters in # converted to Angstrom (the spectra1d spectral unit). - assert label_mouseover.as_text() == ('Pixel x=00010.0 y=00100.0 Value +8.12986e-01', + assert label_mouseover.as_text() == ('Pixel x=00010.0 y=00100.0 Value +8.12986e-01 Jy', 'Wave 1.10000e+05 Angstrom', '') assert label_mouseover.icon == 'c' diff --git a/jdaviz/configs/specviz2d/tests/test_parsers.py b/jdaviz/configs/specviz2d/tests/test_parsers.py index 2282a54431..d6326f77c3 100644 --- a/jdaviz/configs/specviz2d/tests/test_parsers.py +++ b/jdaviz/configs/specviz2d/tests/test_parsers.py @@ -67,7 +67,7 @@ def test_2d_parser_no_unit(specviz2d_helper, mos_spectrum2d): dc_0 = specviz2d_helper.app.data_collection[0] assert dc_0.label == 'my_2d_spec 2D' - assert dc_0.get_component('flux').units == '' + assert dc_0.get_component('flux').units == 'Jy' dc_1 = specviz2d_helper.app.data_collection[1] assert dc_1.label == 'Spectrum 1D' @@ -79,7 +79,7 @@ def test_2d_parser_no_unit(specviz2d_helper, mos_spectrum2d): label_mouseover = specviz2d_helper.app.session.application._tools['g-coords-info'] label_mouseover._viewer_mouse_event(viewer_2d, {'event': 'mousemove', 'domain': {'x': 0, 'y': 0}}) - assert label_mouseover.as_text() == ('Pixel x=00000.0 y=00000.0 Value +3.74540e-01', + assert label_mouseover.as_text() == ('Pixel x=00000.0 y=00000.0 Value +3.74540e-01 Jy', 'Wave 1.00000e-06 m', '') assert label_mouseover.icon == 'a' @@ -90,7 +90,7 @@ def test_2d_parser_no_unit(specviz2d_helper, mos_spectrum2d): {'event': 'mousemove', 'domain': {'x': 7.2e-6, 'y': 3}}) assert label_mouseover.as_text() == ('Cursor 7.20000e-06, 3.00000e+00', 'Wave 7.00000e-06 m (6 pix)', - 'Flux -3.59571e+00') + 'Flux -3.59571e+00 Jy') assert label_mouseover.icon == 'b' From 3e7d1ae5e83f219a71d3769e3bdeaf278740d0e3 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 24 Jan 2025 09:49:11 -0500 Subject: [PATCH 31/32] resolve trace conversion failure --- .../spectral_extraction.py | 20 ++++++------------- .../tests/test_spectral_extraction.py | 3 --- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py index 6e0cfb7ab2..f327070562 100644 --- a/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py @@ -729,29 +729,21 @@ def export_trace(self, add_data=False, **kwargs): # then we're offsetting an existing trace # for FlatTrace, we can keep and expose a new FlatTrace (which has the advantage of # being able to load back into the plugin) - orig_trace = self.trace_trace.get_selected_spectrum( - self.trace_trace.selected_obj, use_display_units=True - ) + orig_trace = self.trace_trace.selected_obj if isinstance(orig_trace, tracing.FlatTrace): - trace = tracing.FlatTrace(self.trace_dataset.get_selected_spectrum( - self.trace_dataset, use_display_units=True), + trace = tracing.FlatTrace(self.trace_dataset.selected_obj, orig_trace.trace_pos+self.trace_offset) else: - trace = tracing.ArrayTrace(self.trace_dataset.get_selected_spectrum( - self.trace_dataset, use_display_units=True), - self.trace_trace.get_selected_spectrum( - self.trace_trace.selected_obj, - use_display_units=True).trace+self.trace_offset) + trace = tracing.ArrayTrace(self.trace_dataset.selected_obj, + self.trace_trace.selected_obj.trace+self.trace_offset) elif self.trace_type_selected == 'Flat': - trace = tracing.FlatTrace(self.trace_dataset.get_selected_spectrum( - use_display_units=True), + trace = tracing.FlatTrace(self.trace_dataset.selected_obj, self.trace_pixel) elif self.trace_type_selected in _model_cls: trace_model = _model_cls[self.trace_type_selected](degree=self.trace_order) - trace = tracing.FitTrace(self.trace_dataset.get_selected_spectrum( - use_display_units=True), + trace = tracing.FitTrace(self.trace_dataset.selected_obj, guess=self.trace_pixel, bins=int(self.trace_bins) if self.trace_do_binning else None, window=self.trace_window, diff --git a/jdaviz/configs/specviz2d/plugins/spectral_extraction/tests/test_spectral_extraction.py b/jdaviz/configs/specviz2d/plugins/spectral_extraction/tests/test_spectral_extraction.py index c04515fdfe..c412c2269a 100644 --- a/jdaviz/configs/specviz2d/plugins/spectral_extraction/tests/test_spectral_extraction.py +++ b/jdaviz/configs/specviz2d/plugins/spectral_extraction/tests/test_spectral_extraction.py @@ -280,9 +280,6 @@ def test_spectral_extraction_flux_unit_conversions(specviz2d_helper, mos_spectru # display units uc.flux_unit.selected = new_flux_unit - exported_trace = pext.export_trace() - assert exported_trace.image._unit == specviz2d_helper.app._get_display_unit('flux') - exported_bg = pext.export_bg() assert exported_bg.image._unit == specviz2d_helper.app._get_display_unit('flux') From d2411c1d68d2ae0a00edcc79ef8a763ca89bf4c6 Mon Sep 17 00:00:00 2001 From: gibsongreen Date: Fri, 24 Jan 2025 15:26:40 -0500 Subject: [PATCH 32/32] address review comments --- jdaviz/configs/mosviz/plugins/tools.py | 13 +++------ .../line_analysis/tests/test_line_analysis.py | 6 ++--- .../spectral_extraction.py | 1 - jdaviz/core/tests/test_tools.py | 27 +++++++++++++++++++ 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/jdaviz/configs/mosviz/plugins/tools.py b/jdaviz/configs/mosviz/plugins/tools.py index a99ab5098c..f7955d8af0 100644 --- a/jdaviz/configs/mosviz/plugins/tools.py +++ b/jdaviz/configs/mosviz/plugins/tools.py @@ -21,18 +21,13 @@ def _is_matched_viewer(self, viewer): def _map_limits(self, from_viewer, to_viewer, limits={}): components = self.viewer.state.data_collection[0]._components # Determine cid for spectral axis - cid = None for key in components.keys(): - if 'Wavelength' in str(key): - cid = str(key) + strkey = str(key) + if 'Wavelength' in strkey or 'Wave' in strkey: + native_unit = u.Unit(self.viewer.state.data_collection[0].get_component(strkey).units) # noqa break - elif 'Wave' in str(key): - cid = str(key) - break - - if cid is not None: - native_unit = u.Unit(self.viewer.state.data_collection[0].get_component(cid).units) else: + # no matches found native_unit = '' current_display_unit = u.Unit(self.viewer.jdaviz_helper.app._get_display_unit('spectral')) diff --git a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py index 7c055378d0..32cc5902e5 100644 --- a/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py +++ b/jdaviz/configs/specviz/plugins/line_analysis/tests/test_line_analysis.py @@ -92,7 +92,7 @@ def test_cubeviz_units(cubeviz_helper, spectrum1d_cube_custom_fluxunit, is in flux/pix2 and flux/sr, and that the results remain consistant between translations of the spectral y axis flux<>surface brightness. """ - cube = spectrum1d_cube_custom_fluxunit(fluxunit=u.MJy / sq_angle_unit, + cube = spectrum1d_cube_custom_fluxunit(fluxunit=u.Jy / sq_angle_unit, shape=(25, 25, 4), with_uncerts=True) cubeviz_helper.load_data(cube, data_label="Test Cube") @@ -119,14 +119,14 @@ def test_cubeviz_units(cubeviz_helper, spectrum1d_cube_custom_fluxunit, la._obj.continuum_marks['right'].y] # change flux unit and make sure result stays the same after conversion - uc.flux_unit.selected = 'Jy' + uc.flux_unit.selected = 'MJy' marks_after = [la._obj.continuum_marks['left'].y, la._obj.continuum_marks['right'].y] # ensure continuum marks update when spectral_y is changed by # multiply converted continuum marks by expected scale factor (MJy -> Jy) - scaling_factor = 1e6 + scaling_factor = 1e-6 assert_allclose([mark * scaling_factor for mark in marks_before], marks_after, rtol=1e-5) # reset to test again after spectral_y_type is changed diff --git a/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py b/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py index f327070562..776981b76f 100644 --- a/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py +++ b/jdaviz/configs/specviz2d/plugins/spectral_extraction/spectral_extraction.py @@ -708,7 +708,6 @@ def import_trace(self, trace): else: # pragma: no cover raise NotImplementedError(f"trace of type {trace.__class__.__name__} not supported") - # UPDATE HERE @with_spinner('trace_spinner') def export_trace(self, add_data=False, **kwargs): """ diff --git a/jdaviz/core/tests/test_tools.py b/jdaviz/core/tests/test_tools.py index beea25e853..ee4e24864d 100644 --- a/jdaviz/core/tests/test_tools.py +++ b/jdaviz/core/tests/test_tools.py @@ -5,6 +5,7 @@ from astropy.nddata import NDData from numpy.testing import assert_allclose from photutils.datasets import make_4gaussians_image +from specutils import Spectrum1D from jdaviz.tests.test_utils import PHOTUTILS_LT_1_12_1 @@ -170,3 +171,29 @@ def test_unit_conversion_limits_update(specviz2d_helper, mos_spectrum2d): # test again when matching viewer's limits are reset assert_allclose(spec_viewer_lims_before, spec_viewer.get_limits()) assert_allclose(spec2d_viewer_lims_before, spec2d_viewer.get_limits()) + + +def test_match_limits_without_wave_component(specviz2d_helper): + data = np.zeros((5, 10)) + spec2d = Spectrum1D(flux=data*u.MJy) + specviz2d_helper.load_data(spec2d) + + spec_viewer = specviz2d_helper.app.get_viewer( + specviz2d_helper.app._jdaviz_helper._default_spectrum_viewer_reference_name) + spec2d_viewer = specviz2d_helper.app.get_viewer( + specviz2d_helper.app._jdaviz_helper._default_spectrum_2d_viewer_reference_name) + + spec_viewer_lims_before = spec_viewer.get_limits() + spec2d_viewer_lims_before = spec2d_viewer.get_limits() + + spec_viewer.reset_limits() + + # ensure limits reset when Wave nor Wavelength component is present + assert_allclose(spec_viewer_lims_before, spec_viewer.get_limits()) + assert_allclose(spec2d_viewer_lims_before, spec2d_viewer.get_limits()) + + spec2d_viewer.reset_limits() + + # test again when when limits are reset with spectrum2d-viewer + assert_allclose(spec_viewer_lims_before, spec_viewer.get_limits()) + assert_allclose(spec2d_viewer_lims_before, spec2d_viewer.get_limits())