Improved GWCS handling in Spectrum1D #1074

rosteen · 2023-08-22T20:01:16Z

This PR allows Spectrum1D to keep the entire GWCS when reading a file (for example a JWST cube), rather than dropping any spatial WCS information as done previously. Probably most applicable beyond just JWST cubes but that's the use case I was targeting off the bat here. This has to go in v2.0 because GWCS doesn't have a swapaxis method like WCS does.

…stropy#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 <keflavich@gmail.com> 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 <erik.tollerud@gmail.com> * Add convenience method Make this a docstring * Add v2.0.0 changelog section --------- Co-authored-by: Erik Tollerud <erik.tollerud@gmail.com>

…mpler, increase computation speed (astropy#1060) * new implementation of flux conserving resample * removed unused method * handle multi dimensional flux inputs * . * Update CHANGES.rst Co-authored-by: Erik Tollerud <erik.tollerud@gmail.com> * omit removing units * added test to compare output to output from running SpectRes --------- Co-authored-by: Erik Tollerud <erik.tollerud@gmail.com>

…just spectral coords

Add changelog heading Remove debugging prints Fix changelog Fix codestyle

codecov · 2023-08-22T20:10:42Z

Codecov Report

Merging #1074 (de78a58) into v2.0-dev (42d1e6e) will increase coverage by 0.23%.
The diff coverage is 94.44%.

@@             Coverage Diff              @@
##           v2.0-dev    #1074      +/-   ##
============================================
+ Coverage     71.03%   71.26%   +0.23%     
============================================
  Files            64       64              
  Lines          4543     4542       -1     
============================================
+ Hits           3227     3237      +10     
+ Misses         1316     1305      -11

Files Changed	Coverage Δ
specutils/spectra/spectrum1d.py	`82.39% <93.54%> (+0.62%)`	⬆️
specutils/io/default_loaders/jwst_reader.py	`58.99% <100.00%> (+1.30%)`	⬆️
...utils/io/default_loaders/tests/test_jwst_reader.py	`77.53% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

cshanahan1 · 2023-08-25T01:10:51Z

specutils/io/default_loaders/jwst_reader.py

@@ -643,13 +644,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


just curious, why was data being transposed before?

Specutils currently demands that the spectral axis be last in a multidimensional spectrum. Specutils 2.0 is going to remove that requirements, so we'll be able to keep the data in the same shape as you would get if you read it in with say astropy.io.fits.

cshanahan1 · 2023-08-25T13:56:32Z

CHANGES.rst

-
-1.11.0 (unreleased)
+
+1.12.0 (unreleased)


needs a change log entry

cshanahan1

looks good, just suggested a possible change to the change log entry to reflect the changes made here.

not essential as most lines are covered in this PR, but a test demonstrating creating a Spectrum1D with a GWCS might be appropriate too.

cshanahan1 · 2023-08-25T18:03:44Z

CHANGES.rst

 Other Changes and Additions
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^

-1.11.0 (unreleased)
+- JWST reader no longer transposes the input data cube for 3D data. [#1074]


correct me if im wrong, this was just my understanding from our convo earlier - but would a better description be that Spectrum1D now allows GWCS as input, and when using the JWST reader to load a Spectrum1D the full GWCS is now loaded?

Good call, I'll update it.

Does that sound better?

rosteen · 2023-08-28T14:33:01Z

I'm going to defer making a test/documentation for creating a Spectrum1D with user-created GWCS, I'll open an issue.

* Allow spectral axis to be anywhere, instead of forcing it to be last (astropy#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 <keflavich@gmail.com> 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 <erik.tollerud@gmail.com> * Add convenience method Make this a docstring * Add v2.0.0 changelog section --------- Co-authored-by: Erik Tollerud <erik.tollerud@gmail.com> * Prepare changelog for 1.10.0 release * Fix Changelog * Fixed issues with ndcube 2.1 docs * Fix incorrect fluxes and uncertainties returned by FluxConservingResampler, increase computation speed (astropy#1060) * new implementation of flux conserving resample * removed unused method * handle multi dimensional flux inputs * . * Update CHANGES.rst Co-authored-by: Erik Tollerud <erik.tollerud@gmail.com> * omit removing units * added test to compare output to output from running SpectRes --------- Co-authored-by: Erik Tollerud <erik.tollerud@gmail.com> * Update changelog for 1.11.0 release * Changelog back to unreleased * Working on retaining full GWCS information in Spectrum1D rather than just spectral coords * Handle getting the spectral axis out of a GWCS Add changelog heading Remove debugging prints Fix changelog Fix codestyle * Add changelog entry * Delete the commented-out old wavelength parsing code * More accurate changelog --------- Co-authored-by: Erik Tollerud <erik.tollerud@gmail.com> Co-authored-by: Nabil Freij <nabil.freij@gmail.com> Co-authored-by: Clare Shanahan <cshanahan@stsci.edu>

rosteen and others added 8 commits August 22, 2023 15:18

Prepare changelog for 1.10.0 release

c168476

Fix Changelog

68cbf24

Fixed issues with ndcube 2.1 docs

9b49fd8

Update changelog for 1.11.0 release

a287776

Changelog back to unreleased

6cc3171

Working on retaining full GWCS information in Spectrum1D rather than …

b6f1c2f

…just spectral coords

rosteen requested review from eteq, nmearl and keflavich as code owners August 22, 2023 20:01

Handle getting the spectral axis out of a GWCS

43cabcc

Add changelog heading Remove debugging prints Fix changelog Fix codestyle

rosteen force-pushed the improved-gwcs-handling branch from 7930be8 to 43cabcc Compare August 22, 2023 20:05

rosteen added this to the v2.0 milestone Aug 22, 2023

rosteen added io data objects Core data objects like Spectrum1D or SpectralCollection labels Aug 22, 2023

cshanahan1 reviewed Aug 25, 2023

View reviewed changes

rosteen added 2 commits August 25, 2023 10:24

Add changelog entry

89d3611

Delete the commented-out old wavelength parsing code

a3a6535

cshanahan1 approved these changes Aug 25, 2023

View reviewed changes

More accurate changelog

de78a58

rosteen merged commit 4041081 into astropy:v2.0-dev Aug 28, 2023
9 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved GWCS handling in Spectrum1D #1074

Improved GWCS handling in Spectrum1D #1074

rosteen commented Aug 22, 2023

codecov bot commented Aug 22, 2023 •

edited

cshanahan1 Aug 25, 2023

rosteen Aug 25, 2023

cshanahan1 Aug 25, 2023

cshanahan1 left a comment

cshanahan1 Aug 25, 2023

rosteen Aug 25, 2023

rosteen Aug 25, 2023

rosteen commented Aug 28, 2023

Improved GWCS handling in Spectrum1D #1074

Improved GWCS handling in Spectrum1D #1074

Conversation

rosteen commented Aug 22, 2023

codecov bot commented Aug 22, 2023 • edited

Codecov Report

cshanahan1 Aug 25, 2023

Choose a reason for hiding this comment

rosteen Aug 25, 2023

Choose a reason for hiding this comment

cshanahan1 Aug 25, 2023

Choose a reason for hiding this comment

cshanahan1 left a comment

Choose a reason for hiding this comment

cshanahan1 Aug 25, 2023

Choose a reason for hiding this comment

rosteen Aug 25, 2023

Choose a reason for hiding this comment

rosteen Aug 25, 2023

Choose a reason for hiding this comment

rosteen commented Aug 28, 2023

codecov bot commented Aug 22, 2023 •

edited