From b1a3031fe88595630b3369aec9db4f233e50d7a1 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Tue, 24 Sep 2024 17:15:38 -0400 Subject: [PATCH 1/5] BUG: Fix wrong Subset unit in Specviz2d --- jdaviz/app.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index 17c0981c2d..df0330bc21 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -14,7 +14,7 @@ from ipysplitpanes import SplitPanes import matplotlib.cm as cm import numpy as np -from glue.config import colormaps, settings as glue_settings +from glue.config import colormaps, data_translator, settings as glue_settings from glue.core import HubListener from glue.core.link_helpers import LinkSame, LinkSameWithUnits from glue.core.message import (DataCollectionAddMessage, @@ -1067,13 +1067,15 @@ def _get_range_subset_bounds(self, subset_state, simplify_spectral=True, use_display_units=False): # TODO: Use global display units # units = dc[0].data.coords.spectral_axis.unit - viewer = self.get_viewer(self._jdaviz_helper. _default_spectrum_viewer_reference_name) - data = viewer.data() - if data and len(data) > 0 and isinstance(data[0], Spectrum1D): - units = data[0].spectral_axis.unit + data = subset_state.att.parent + ndim = data.get_component("flux").ndim + if ndim == 2: + units = u.pix else: - raise ValueError("Unable to find spectral axis units") - if use_display_units: + handler, _ = data_translator.get_handler_for(Spectrum1D) + spec = handler.to_object(data) + units = spec.spectral_axis.unit + if use_display_units and units != u.pix: # converting may result in flipping order (wavelength <-> frequency) ret_units = self._get_display_unit('spectral') subset_bounds = [(subset_state.lo * units).to(ret_units, u.spectral()), From 9dc6d6ee47fb5b35409d7c26693b1d88c8bb9250 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Tue, 24 Sep 2024 17:45:54 -0400 Subject: [PATCH 2/5] Add test and change log --- CHANGES.rst | 2 ++ .../configs/specviz2d/tests/test_parsers.py | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index b371412871..a447f8324e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -254,6 +254,8 @@ Specviz Specviz2d ^^^^^^^^^ +- Fixed Subset unit when it is created in 2D spectrum viewer. [#3201] + 3.10.3 (2024-07-22) =================== diff --git a/jdaviz/configs/specviz2d/tests/test_parsers.py b/jdaviz/configs/specviz2d/tests/test_parsers.py index 4a9d1b2233..30a9840d12 100644 --- a/jdaviz/configs/specviz2d/tests/test_parsers.py +++ b/jdaviz/configs/specviz2d/tests/test_parsers.py @@ -1,7 +1,10 @@ import pytest import stdatamodels +from astropy import units as u from astropy.io import fits from astropy.utils.data import download_file +from glue.core.edit_subset_mode import NewMode +from glue.core.roi import XRangeROI from jdaviz.utils import PRIHDR_KEY from jdaviz.configs.imviz.tests.utils import create_example_gwcs @@ -134,6 +137,25 @@ def test_2d_1d_parser(specviz2d_helper, mos_spectrum2d, spectrum1d): specviz2d_helper.load_data(spectrum_2d=mos_spectrum2d, spectrum_1d=spectrum1d) assert specviz2d_helper.app.data_collection.labels == ['Spectrum 2D', 'Spectrum 1D'] + spec2d_viewer = specviz2d_helper.app.get_viewer("spectrum-2d-viewer") + assert spec2d_viewer.figure.axes[0].label == "x: pixels" # -0.5, 14.5 + spec2d_viewer.apply_roi(XRangeROI(10, 12)) + + spec2d_viewer.session.edit_subset_mode._mode = NewMode + + spec1d_viewer = specviz2d_helper.app.get_viewer("spectrum-viewer") + assert spec1d_viewer.figure.axes[0].label == "Wavelength [Angstrom]" # 6000, 8000 + spec1d_viewer.apply_roi(XRangeROI(7000, 7100)) + + # Subset 1 should follow Spectrum 2D viewer unit. + # Subset 2 should follow Spectrum 1D viewer unit. + subsets = specviz2d_helper.app.get_subsets() + assert len(subsets) == 2 + s1 = subsets["Subset 1"] + s2 = subsets["Subset 2"] + assert s1.lower.unit == s1.upper.unit == u.pix + assert s2.lower.unit == s2.upper.unit == u.AA + def test_parser_no_data(specviz2d_helper): with pytest.raises(ValueError, match='Must provide spectrum_2d or spectrum_1d'): From fe250fa570c2e47c4bfe3a294083879fd09f2b4d Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Tue, 24 Sep 2024 18:55:22 -0400 Subject: [PATCH 3/5] cubeviz is so special and fixed an ambiguous specviz2d test, maybe --- jdaviz/app.py | 23 ++++++++++++++++------- jdaviz/tests/test_subsets.py | 2 +- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index df0330bc21..ed1bb8550e 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -1067,14 +1067,23 @@ def _get_range_subset_bounds(self, subset_state, simplify_spectral=True, use_display_units=False): # TODO: Use global display units # units = dc[0].data.coords.spectral_axis.unit - data = subset_state.att.parent - ndim = data.get_component("flux").ndim - if ndim == 2: - units = u.pix + if self.config == "cubeviz": + viewer = self.get_viewer(self._jdaviz_helper._default_spectrum_viewer_reference_name) + data = viewer.data() + if data and len(data) > 0 and isinstance(data[0], Spectrum1D): + units = data[0].spectral_axis.unit + else: + raise ValueError("Unable to find spectral axis units") else: - handler, _ = data_translator.get_handler_for(Spectrum1D) - spec = handler.to_object(data) - units = spec.spectral_axis.unit + data = subset_state.att.parent + ndim = data.get_component("flux").ndim + if ndim == 2: + units = u.pix + else: + handler, _ = data_translator.get_handler_for(Spectrum1D) + spec = handler.to_object(data) + units = spec.spectral_axis.unit + if use_display_units and units != u.pix: # converting may result in flipping order (wavelength <-> frequency) ret_units = self._get_display_unit('spectral') diff --git a/jdaviz/tests/test_subsets.py b/jdaviz/tests/test_subsets.py index 6f0bec6ce9..ed227dc931 100644 --- a/jdaviz/tests/test_subsets.py +++ b/jdaviz/tests/test_subsets.py @@ -690,7 +690,7 @@ def test_only_overlapping_spectral_regions(specviz_helper, spectrum1d): def test_overlapping_in_specviz2d(specviz2d_helper, mos_spectrum2d): specviz2d_helper.load_data(spectrum_2d=mos_spectrum2d) viewer = specviz2d_helper.app.get_viewer( - specviz2d_helper._default_spectrum_2d_viewer_reference_name) + specviz2d_helper._default_spectrum_viewer_reference_name) viewer.apply_roi(XRangeROI(6400, 7400)) specviz2d_helper.app.session.edit_subset_mode.mode = AndNotMode From e4d6311b8a6006ff17919cf6a5653a4ab2669c00 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:46:05 -0400 Subject: [PATCH 4/5] Address comment from kecnry --- jdaviz/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jdaviz/app.py b/jdaviz/app.py index ed1bb8550e..806790d96b 100644 --- a/jdaviz/app.py +++ b/jdaviz/app.py @@ -1067,7 +1067,7 @@ def _get_range_subset_bounds(self, subset_state, simplify_spectral=True, use_display_units=False): # TODO: Use global display units # units = dc[0].data.coords.spectral_axis.unit - if self.config == "cubeviz": + if not hasattr(subset_state.att, "parent"): # e.g., Cubeviz viewer = self.get_viewer(self._jdaviz_helper._default_spectrum_viewer_reference_name) data = viewer.data() if data and len(data) > 0 and isinstance(data[0], Spectrum1D): From eafb391a6213d0d51e8ce247cd7e4ab519a28261 Mon Sep 17 00:00:00 2001 From: "P. L. Lim" <2090236+pllim@users.noreply.github.com> Date: Tue, 1 Oct 2024 09:37:06 -0400 Subject: [PATCH 5/5] Move change log to match new milestone --- CHANGES.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1b5da2cfca..2db2dbec97 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -162,6 +162,8 @@ Specviz Specviz2d ^^^^^^^^^ +- Fixed Subset unit when it is created in 2D spectrum viewer. [#3201] + Other Changes and Additions --------------------------- @@ -260,8 +262,6 @@ Specviz Specviz2d ^^^^^^^^^ -- Fixed Subset unit when it is created in 2D spectrum viewer. [#3201] - 3.10.3 (2024-07-22) ===================