From 75b1dd9b7f63048ba72ef9f590b2b09d5d571cbf Mon Sep 17 00:00:00 2001 From: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> Date: Mon, 27 Mar 2023 12:16:41 -0400 Subject: [PATCH 01/12] Allow spectral axis to be anywhere, instead of forcing it to be last (#1033) * Starting to work on flexible spectral axis location Debugging initial spectrum creation Set private attribute here Working on debugging failing tests More things are temporarily broken, but I don't want to lose this work so I'm committing here Set spectral axis index to 0 if flux is None Working through test failures Fix codestyle Allow passing spectral_axis_index to wcs_fits loader Require specification of spectral_axis_index if WCS is 1D and flux is multi-D Decrement spectral_axis_index when slicing with integers Propagate spectral_axis_index through resampling Fix last test to account for spectral axis staying first Fix codestyle Specify spectral_axis_index in SDSS plate loader Greatly simply extract_bounding_spectral_region Account for variable spectral axis location in moment calculation, fix doc example Working on SpectrumCollection moment handling...not sure this is the way Need to add one to the axis index here Update narrative docs to reflect updates * Add back in the option to move the spectral axis to last, for back-compatibility Work around pixel unit slicing failure Change order on crop example Fix spectral slice handling in tuple input case (e.g. crop) Update output of crop example * Apply suggestions from code review Co-authored-by: Adam Ginsburg Apply suggestion from code review Add helpful comment * Address review comment about move_spectral_axis, more docs * Add suggested line to docstring Co-authored-by: Erik Tollerud * Add convenience method Make this a docstring * Add v2.0.0 changelog section --------- Co-authored-by: Erik Tollerud --- docs/conf.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/conf.py b/docs/conf.py index 1d45a0bda..4ae6952fd 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -62,6 +62,7 @@ intersphinx_mapping['reproject'] = ('https://reproject.readthedocs.io/en/stable/', None) intersphinx_mapping['mpl_animators'] = ('https://docs.sunpy.org/projects/mpl-animators/en/stable/', None) + # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. exclude_patterns.append('_templates') From c1684763d5175b596d8d9763ab5408e5720cf9af Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Wed, 5 Apr 2023 17:14:11 -0400 Subject: [PATCH 02/12] Prepare changelog for 1.10.0 release --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index e19329e32..b3cf86d8b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,7 +10,7 @@ New Features Other Changes and Additions ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -1.11.0 (unreleased) +1.12.0 (unreleased) ------------------- New Features From 68cbf249351ba46bcbcab817b3a54690ecb97629 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Tue, 15 Aug 2023 12:59:18 -0400 Subject: [PATCH 03/12] Fix Changelog --- CHANGES.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index b3cf86d8b..5cc4a6510 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,11 +1,17 @@ 2.0.0 (unreleased) ------------------ +- Spectral axis can now be any axis, rather than being forced to be last. See docs + for more details. [#1033] + +1.11.0 (Unreleased) +------------------- + New Features ^^^^^^^^^^^^ -- Spectral axis can now be any axis, rather than being forced to be last. See docs - for more details. [#1033] +Bug Fixes +^^^^^^^^^ Other Changes and Additions ^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 9b49fd8ed99d2f939f6034351f8ae7e90b5a6046 Mon Sep 17 00:00:00 2001 From: Nabil Freij Date: Tue, 14 Mar 2023 19:31:55 -0700 Subject: [PATCH 04/12] Fixed issues with ndcube 2.1 docs --- docs/conf.py | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index 4ae6952fd..1d45a0bda 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -62,7 +62,6 @@ intersphinx_mapping['reproject'] = ('https://reproject.readthedocs.io/en/stable/', None) intersphinx_mapping['mpl_animators'] = ('https://docs.sunpy.org/projects/mpl-animators/en/stable/', None) - # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. exclude_patterns.append('_templates') From 0c97cbadc525c2f8b4c7a0efa3d42699f3d7ddd5 Mon Sep 17 00:00:00 2001 From: Clare Shanahan Date: Fri, 2 Jun 2023 08:55:34 -0400 Subject: [PATCH 05/12] Fix incorrect fluxes and uncertainties returned by FluxConservingResampler, increase computation speed (#1060) * new implementation of flux conserving resample * removed unused method * handle multi dimensional flux inputs * . * Update CHANGES.rst Co-authored-by: Erik Tollerud * omit removing units * added test to compare output to output from running SpectRes --------- Co-authored-by: Erik Tollerud --- CHANGES.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.rst b/CHANGES.rst index 5cc4a6510..563e88e39 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,6 +12,7 @@ New Features Bug Fixes ^^^^^^^^^ +- Reimplementation of FluxConservingResampler. It is now faster and yields more accurate results. [#1060] Other Changes and Additions ^^^^^^^^^^^^^^^^^^^^^^^^^^^ From a287776379fdf7c8d38c9a52fba6287220dc5eaa Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Fri, 16 Jun 2023 17:15:23 -0400 Subject: [PATCH 06/12] Update changelog for 1.11.0 release --- CHANGES.rst | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 563e88e39..027a10d3b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,34 +1,10 @@ + 2.0.0 (unreleased) ------------------ - Spectral axis can now be any axis, rather than being forced to be last. See docs for more details. [#1033] -1.11.0 (Unreleased) -------------------- - -New Features -^^^^^^^^^^^^ - -Bug Fixes -^^^^^^^^^ -- Reimplementation of FluxConservingResampler. It is now faster and yields more accurate results. [#1060] - -Other Changes and Additions -^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -1.12.0 (unreleased) -------------------- - -New Features -^^^^^^^^^^^^ - -Bug Fixes -^^^^^^^^^ - -Other Changes and Additions -^^^^^^^^^^^^^^^^^^^^^^^^^^^ - 1.11.0 (2023-06-16) ------------------- From 6cc3171e09cc5b25d4206ccb6fee07b7d4d5c4fd Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Fri, 16 Jun 2023 17:25:12 -0400 Subject: [PATCH 07/12] Changelog back to unreleased --- CHANGES.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 027a10d3b..3971338ff 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,18 @@ - Spectral axis can now be any axis, rather than being forced to be last. See docs for more details. [#1033] + +1.12.0 (unreleased) +------------------- + +New Features +^^^^^^^^^^^^ + +Bug Fixes +^^^^^^^^^ + +Other Changes and Additions +^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1.11.0 (2023-06-16) ------------------- From b6f1c2f2fafb4b0174fd5a1b4c39d381c004951a Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Tue, 15 Aug 2023 17:32:08 -0400 Subject: [PATCH 08/12] Working on retaining full GWCS information in Spectrum1D rather than just spectral coords --- specutils/io/default_loaders/jwst_reader.py | 18 +++++----- specutils/spectra/spectrum1d.py | 40 +++++++++++++-------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/specutils/io/default_loaders/jwst_reader.py b/specutils/io/default_loaders/jwst_reader.py index b515de198..82b4bd115 100644 --- a/specutils/io/default_loaders/jwst_reader.py +++ b/specutils/io/default_loaders/jwst_reader.py @@ -583,10 +583,12 @@ def _jwst_s3d_loader(filename, **kwargs): except (ValueError, KeyError): flux_unit = None - # The spectral axis is first. We need it last - flux_array = hdu.data.T + flux_array = hdu.data flux = Quantity(flux_array, unit=flux_unit) + ''' + # I think this can all be removed now, will test + # Get the wavelength array from the GWCS object which returns a # tuple of (RA, Dec, lambda). # Since the spatial and spectral axes are orthogonal in s3d data, @@ -614,6 +616,7 @@ def _jwst_s3d_loader(filename, **kwargs): wcs = WCS(hdu.header) # Swap to match the flux transpose wcs = wcs.swapaxes(-1, 0) + ''' # Merge primary and slit headers and dump into meta slit_header = hdu.header @@ -625,7 +628,7 @@ def _jwst_s3d_loader(filename, **kwargs): ext_name = primary_header.get("ERREXT", "ERR") err_type = hdulist[ext_name].header.get("ERRTYPE", 'ERR') err_unit = hdulist[ext_name].header.get("BUNIT", None) - err_array = hdulist[ext_name].data.T + err_array = hdulist[ext_name].data # ERRTYPE can be one of "ERR", "IERR", "VAR", "IVAR" # but mostly ERR for JWST cubes @@ -643,13 +646,10 @@ def _jwst_s3d_loader(filename, **kwargs): # get mask information mask_name = primary_header.get("MASKEXT", "DQ") - mask = hdulist[mask_name].data.T + mask = hdulist[mask_name].data + + spec = Spectrum1D(flux=flux, wcs=wcs, meta=meta, uncertainty=err, mask=mask, spectral_axis_index=0) - if wcs is not None: - spec = Spectrum1D(flux=flux, wcs=wcs, meta=meta, uncertainty=err, mask=mask) - else: - spec = Spectrum1D(flux=flux, spectral_axis=wavelength, meta=meta, - uncertainty=err, mask=mask) spectra.append(spec) return SpectrumList(spectra) diff --git a/specutils/spectra/spectrum1d.py b/specutils/spectra/spectrum1d.py index 7348b7a43..78410998c 100644 --- a/specutils/spectra/spectrum1d.py +++ b/specutils/spectra/spectrum1d.py @@ -6,6 +6,7 @@ from astropy.utils.decorators import lazyproperty from astropy.utils.decorators import deprecated from astropy.nddata import NDUncertainty, NDIOMixin, NDArithmeticMixin +from gwcs.wcs import WCS as GWCS from .spectral_axis import SpectralAxis from .spectrum_mixin import OneDSpectrumMixin @@ -228,24 +229,34 @@ def __init__(self, flux=None, spectral_axis=None, spectral_axis_index=None, f"of the corresponding flux axis ({flux.shape[self.spectral_axis_index]})") # If a WCS is provided, determine which axis is the spectral axis - if wcs is not None and hasattr(wcs, "naxis"): - if wcs.naxis > 1: + if wcs is not None: + naxis = None + if hasattr(wcs, "naxis"): + naxis = wcs.naxis + # GWCS doesn't have naxis + elif hasattr(wcs, "world_n_dim"): + naxis = wcs.world_n_dim + + if naxis is not None and naxis > 1: temp_axes = [] phys_axes = wcs.world_axis_physical_types - for i in range(len(phys_axes)): - if phys_axes[i] is None: - continue - if phys_axes[i][0:2] == "em" or phys_axes[i][0:5] == "spect": - temp_axes.append(i) - if len(temp_axes) != 1: - raise ValueError("Input WCS must have exactly one axis with " - "spectral units, found {}".format(len(temp_axes))) - else: - # Due to FITS conventions, the WCS axes are listed in opposite - # order compared to the data array. - self._spectral_axis_index = len(flux.shape)-temp_axes[0]-1 + if self._spectral_axis_index is None: + for i in range(len(phys_axes)): + if phys_axes[i] is None: + continue + if phys_axes[i][0:2] == "em" or phys_axes[i][0:5] == "spect": + temp_axes.append(i) + if len(temp_axes) != 1: + raise ValueError("Input WCS must have exactly one axis with " + "spectral units, found {}".format(len(temp_axes))) + else: + # Due to FITS conventions, the WCS axes are listed in opposite + # order compared to the data array. + self._spectral_axis_index = len(flux.shape)-temp_axes[0]-1 if move_spectral_axis is not None: + if isinstance(wcs, GWCS): + raise ValueError("move_spectral_axis cannot be used with GWCS") if isinstance(move_spectral_axis, str): if move_spectral_axis.lower() == 'first': move_to_index = 0 @@ -280,6 +291,7 @@ def __init__(self, flux=None, spectral_axis=None, spectral_axis_index=None, self._spectral_axis_index, move_to_index) self._spectral_axis_index = move_to_index + print(f"Spectral axis index is {self._spectral_axis_index}") else: if flux is not None and flux.ndim == 1: From 43cabcc6b9a26cf0fd1587f56569171f0cd0ada0 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Tue, 22 Aug 2023 15:04:58 -0400 Subject: [PATCH 09/12] Handle getting the spectral axis out of a GWCS Add changelog heading Remove debugging prints Fix changelog Fix codestyle --- CHANGES.rst | 4 +++- specutils/io/default_loaders/jwst_reader.py | 2 -- .../default_loaders/tests/test_jwst_reader.py | 2 +- specutils/spectra/spectrum1d.py | 21 ++++++++++++++++--- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3971338ff..f7bb04f7b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,9 @@ - 2.0.0 (unreleased) ------------------ +New Features +^^^^^^^^^^^^ + - Spectral axis can now be any axis, rather than being forced to be last. See docs for more details. [#1033] diff --git a/specutils/io/default_loaders/jwst_reader.py b/specutils/io/default_loaders/jwst_reader.py index 82b4bd115..6c49d41f5 100644 --- a/specutils/io/default_loaders/jwst_reader.py +++ b/specutils/io/default_loaders/jwst_reader.py @@ -8,8 +8,6 @@ from astropy.table import Table from astropy.io import fits from astropy.nddata import StdDevUncertainty, VarianceUncertainty, InverseVariance -from astropy.time import Time -from astropy.wcs import WCS from gwcs.wcstools import grid_from_bounding_box from ...spectra import Spectrum1D, SpectrumList diff --git a/specutils/io/default_loaders/tests/test_jwst_reader.py b/specutils/io/default_loaders/tests/test_jwst_reader.py index cbb515fd7..6f1ddf0c3 100644 --- a/specutils/io/default_loaders/tests/test_jwst_reader.py +++ b/specutils/io/default_loaders/tests/test_jwst_reader.py @@ -434,7 +434,7 @@ def test_jwst_s3d_single(tmp_path, cube): data = Spectrum1D.read(tmpfile, format='JWST s3d') assert type(data) is Spectrum1D - assert data.shape == (10, 10, 30) + assert data.shape == (30, 10, 10) assert data.uncertainty is not None assert data.mask is not None assert data.uncertainty.unit == 'MJy' diff --git a/specutils/spectra/spectrum1d.py b/specutils/spectra/spectrum1d.py index 78410998c..b2d565b4a 100644 --- a/specutils/spectra/spectrum1d.py +++ b/specutils/spectra/spectrum1d.py @@ -3,6 +3,7 @@ import numpy as np from astropy import units as u +from astropy.coordinates import SpectralCoord from astropy.utils.decorators import lazyproperty from astropy.utils.decorators import deprecated from astropy.nddata import NDUncertainty, NDIOMixin, NDArithmeticMixin @@ -291,7 +292,6 @@ def __init__(self, flux=None, spectral_axis=None, spectral_axis_index=None, self._spectral_axis_index, move_to_index) self._spectral_axis_index = move_to_index - print(f"Spectral axis index is {self._spectral_axis_index}") else: if flux is not None and flux.ndim == 1: @@ -365,8 +365,23 @@ def __init__(self, flux=None, spectral_axis=None, spectral_axis_index=None, spec_axis = self.wcs.spectral.pixel_to_world( np.arange(self.flux.shape[self.spectral_axis_index])) else: - spec_axis = self.wcs.pixel_to_world( - np.arange(self.flux.shape[self.spectral_axis_index])) + # We now keep the entire GWCS, including spatial information, so we need to include + # all axes in the pixel_to_world call. Note that this assumes/requires that the + # dispersion is the same at all spatial locations. + wcs_args = [] + for i in range(len(self.flux.shape)): + wcs_args.append(np.zeros(self.flux.shape[self.spectral_axis_index])) + # Replace with arange for the spectral axis + wcs_args[self.spectral_axis_index] = np.arange(self.flux.shape[self.spectral_axis_index]) + wcs_args.reverse() + temp_coords = self.wcs.pixel_to_world(*wcs_args) + # If there are spatial axes, temp_coords will have a SkyCoord and a SpectralCoord + if isinstance(temp_coords, list): + for coords in temp_coords: + if isinstance(coords, SpectralCoord): + spec_axis = coords + else: + spec_axis = temp_coords try: if spec_axis.unit.is_equivalent(u.one): From 89d36113c16c9bc4b9d40290c7ef7be1d8028e8c Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Fri, 25 Aug 2023 10:24:21 -0400 Subject: [PATCH 10/12] Add changelog entry --- CHANGES.rst | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index f7bb04f7b..e411b4292 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,7 +6,15 @@ New Features - Spectral axis can now be any axis, rather than being forced to be last. See docs for more details. [#1033] - + +- Spectrum1D now keeps the entire input GWCS instead of extracting only the spectral + information. [#1074] + +Other Changes and Additions +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- JWST reader no longer transposes the input data cube for 3D data. [#1074] + 1.12.0 (unreleased) ------------------- From a3a6535a16ba10c0c1381b01ff0f0de53dbf5341 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Fri, 25 Aug 2023 11:18:51 -0400 Subject: [PATCH 11/12] Delete the commented-out old wavelength parsing code --- specutils/io/default_loaders/jwst_reader.py | 32 --------------------- 1 file changed, 32 deletions(-) diff --git a/specutils/io/default_loaders/jwst_reader.py b/specutils/io/default_loaders/jwst_reader.py index 6c49d41f5..780ab090e 100644 --- a/specutils/io/default_loaders/jwst_reader.py +++ b/specutils/io/default_loaders/jwst_reader.py @@ -584,38 +584,6 @@ def _jwst_s3d_loader(filename, **kwargs): flux_array = hdu.data flux = Quantity(flux_array, unit=flux_unit) - ''' - # I think this can all be removed now, will test - - # Get the wavelength array from the GWCS object which returns a - # tuple of (RA, Dec, lambda). - # Since the spatial and spectral axes are orthogonal in s3d data, - # it is much faster to compute a slice down the spectral axis. - grid = grid_from_bounding_box(wcs.bounding_box)[:, :, 0, 0] - _, _, wavelength_array = wcs(*grid) - _, _, wavelength_unit = wcs.output_frame.unit - - wavelength = Quantity(wavelength_array, unit=wavelength_unit) - - # The GWCS is currently broken for some IFUs, here we work around that - wcs = None - if wavelength.shape[0] != flux.shape[-1]: - # Need MJD-OBS for this workaround - if 'MJD-OBS' not in hdu.header: - for key in ('MJD-BEG', 'DATE-OBS'): # Possible alternatives - if key in hdu.header: - if key.startswith('MJD'): - hdu.header['MJD-OBS'] = hdu.header[key] - break - else: - t = Time(hdu.header[key]) - hdu.header['MJD-OBS'] = t.mjd - break - wcs = WCS(hdu.header) - # Swap to match the flux transpose - wcs = wcs.swapaxes(-1, 0) - ''' - # Merge primary and slit headers and dump into meta slit_header = hdu.header header = primary_header.copy() From de78a586bc8495278e17366fcaf5a3c7a466dd11 Mon Sep 17 00:00:00 2001 From: Ricky O'Steen Date: Fri, 25 Aug 2023 15:43:18 -0400 Subject: [PATCH 12/12] More accurate changelog --- CHANGES.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index e411b4292..30da0e118 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,13 +7,13 @@ New Features - Spectral axis can now be any axis, rather than being forced to be last. See docs for more details. [#1033] -- Spectrum1D now keeps the entire input GWCS instead of extracting only the spectral - information. [#1074] +- Spectrum1D now properly handles GWCS input for wcs attribute. [#1074] Other Changes and Additions ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- JWST reader no longer transposes the input data cube for 3D data. [#1074] +- JWST reader no longer transposes the input data cube for 3D data and retains + full GWCS information (including spatial). [#1074] 1.12.0 (unreleased) -------------------