From ff6c58f76734aefada8fbd30abd52eb592b8e526 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Tue, 8 Oct 2024 14:05:21 -0400 Subject: [PATCH 1/3] Fix unit handling in continuum and moment calculations --- CHANGES.rst | 2 +- .../cubeviz/plugins/moment_maps/moment_maps.py | 15 +++++++++++++-- jdaviz/core/template_mixin.py | 6 ++++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index e81f94ce9f..dd8a97c006 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,7 +6,7 @@ New Features - Added flux/surface brightness translation and surface brightness unit conversion in Cubeviz and Specviz. [#2781, #2940, #3088, #3111, #3113, #3129, - #3139, #3149, #3155, #3178, #3185, #3187, #3190, #3156, #3200, #3192, #3206, #3211] + #3139, #3149, #3155, #3178, #3185, #3187, #3190, #3156, #3200, #3192, #3206, #3211, #3216] - Plugin tray is now open by default. [#2892] diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py index 66965d80ef..ca4a099d1c 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/moment_maps.py @@ -271,7 +271,13 @@ def calculate_moment(self, add_data=True): # slice out desired region # TODO: should we add a warning for a composite spectral subset? - spec_reg = self.spectral_subset.selected_obj + if self.spectral_subset.selected == "Entire Spectrum": + spec_reg = None + else: + spec_reg = self.app.get_subsets(self.spectral_subset.selected, + simplify_spectral=True, + use_display_units=True) + # We need to convert the spectral region to the display units if spec_reg is None: slab = cube @@ -303,7 +309,12 @@ def calculate_moment(self, add_data=True): ref_wavelength = self.reference_wavelength * u.Unit(self.dataset_spectral_unit) slab_sa = slab.spectral_axis.to("km/s", doppler_convention="relativistic", doppler_rest=ref_wavelength) - slab = Spectrum1D(slab.flux, slab_sa) + slab = Spectrum1D(slab.flux, slab_sa, uncertainty=slab.uncertainty) + # Otherwise convert spectral axis to display units, have to do frequency <-> wavelength + # before calculating + else: + slab_sa = slab.spectral_axis.to(self.app._get_display_unit('spectral')) + slab = Spectrum1D(slab.flux, slab_sa, uncertainty=slab.uncertainty) # Finally actually calculate the moment self.moment = analysis.moment(slab, order=n_moment).T diff --git a/jdaviz/core/template_mixin.py b/jdaviz/core/template_mixin.py index 5614652cd9..a3db290531 100644 --- a/jdaviz/core/template_mixin.py +++ b/jdaviz/core/template_mixin.py @@ -2970,7 +2970,8 @@ def _get_continuum(self, dataset, spectral_subset, update_marks=False, per_pixel if per_pixel: if self.app.config != 'cubeviz': raise ValueError("per-pixel only supported for cubeviz") - full_spectrum = self.app._jdaviz_helper.get_data(self.dataset.selected) + full_spectrum = self.app._jdaviz_helper.get_data(self.dataset.selected, + use_display_units=True) else: full_spectrum = dataset.get_selected_spectrum(use_display_units=True) @@ -2994,7 +2995,8 @@ def _get_continuum(self, dataset, spectral_subset, update_marks=False, per_pixel spectrum = full_spectrum else: sr = self.app.get_subsets(spectral_subset.selected, - simplify_spectral=True) + simplify_spectral=True, + use_display_units=True) spectrum = extract_region(full_spectrum, sr, return_single_spectrum=True) sr_lower = np.nanmin(spectrum.spectral_axis[spectrum.spectral_axis >= sr.lower]) # noqa sr_upper = np.nanmax(spectrum.spectral_axis[spectrum.spectral_axis <= sr.upper]) # noqa From 5270ce448a4838657df59f507e356674bab91cdb Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Tue, 8 Oct 2024 14:18:05 -0400 Subject: [PATCH 2/3] Add test that fails on main --- .../cubeviz/plugins/moment_maps/tests/test_moment_maps.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py index 06bd361c20..4448193356 100644 --- a/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py +++ b/jdaviz/configs/cubeviz/plugins/moment_maps/tests/test_moment_maps.py @@ -7,6 +7,7 @@ from astropy import units as u from astropy.io import fits from astropy.nddata import CCDData +from astropy.tests.helper import assert_quantity_allclose from astropy.wcs import WCS from numpy.testing import assert_allclose from specutils import SpectralRegion @@ -244,9 +245,12 @@ def test_moment_frequency_unit_conversion(cubeviz_helper, spectrum1d_cube_larger mm.n_moment = 1 mm.output_unit = 'Spectral Unit' moment_1_data = mm.calculate_moment() + mm.n_moment = 0 + moment_0_data = mm.calculate_moment() # Check to make sure there are no nans assert len(np.where(moment_1_data.data > 0)[0]) == 8 + assert_quantity_allclose(moment_0_data, -2.9607526e+09*u.Unit("Hz Jy / pix2")) def test_write_momentmap(cubeviz_helper, spectrum1d_cube, tmp_path): From 7911b268c921dbdacab490b085ccc005debcabdb Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Wed, 16 Oct 2024 15:25:42 -0400 Subject: [PATCH 3/3] Bump specutils pin --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 39a7adfc47..d8ea906444 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,7 +23,7 @@ dependencies = [ "ipywidgets>=8.0.6", "solara>=1.39.0", "pyyaml>=5.4.1", - "specutils>=1.16", + "specutils>=1.18", "specreduce>=1.4.1", "photutils>=1.4", "glue-astronomy>=0.10",