Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

BUG: Fix wrong Subset unit when created in 2D spectrum viewer #3201

Merged
merged 6 commits into from
Oct 1, 2024
Merged

BUG: Fix wrong Subset unit when created in 2D spectrum viewer #3201

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b1a3031
BUG: Fix wrong Subset unit in Specviz2d
pllim Sep 24, 2024
9dc6d6e
Add test and change log
pllim Sep 24, 2024
fe250fa
cubeviz is so special
pllim Sep 24, 2024
e4d6311
Address comment from kecnry
pllim Sep 30, 2024
3e501c0
Merge branch 'main' into specviz2d-subset-unit
pllim Sep 30, 2024
eafb391
Move change log to match new milestone
pllim Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ Specviz
Specviz2d
^^^^^^^^^

- Fixed Subset unit when it is created in 2D spectrum viewer. [#3201]

3.10.3 (2024-07-22)
===================

Expand Down
16 changes: 9 additions & 7 deletions jdaviz/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()),
Expand Down
22 changes: 22 additions & 0 deletions jdaviz/configs/specviz2d/tests/test_parsers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could have used the API from #3104 but since it is not merged yet, I didn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3104 is now merged but the API is not yet public. Should I keep this as-is or should I use the helper.plugins['Subset Tools']._obj.import_region(...) syntax here?


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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that this line throws exception on main without this patch, but passes with this patch. Exception on main:

>       assert s1.lower.unit == s1.upper.unit == u.pix
E       assert Unit("Angstrom") == Unit("pix")
E        +  where Unit("Angstrom") = <Quantity 12. Angstrom>.unit
E        +    where <Quantity 12. Angstrom> = Spectral Region, 1 sub-regions:\n  (10.0 Angstrom, 12.0 Angstrom) .upper
E        +  and   Unit("pix") = 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'):
Expand Down
Loading