1D Wavelength Calibration

[hpparvi]

This PR introduces a new 1D wavelength calibration class, specreduce.wavecal1d.WavelengthCalibration1D, intended to fully replace the existing specreduce.wavelength_calibration.WavelengthCalibration1D in version 2.0 (I'm keeping the previous class for backwards compatibility).

The only (somewhat) significant feature currently missing is support for generating standard spectroscopic -GRI and -GRA WCSs as described in Greisen et al. (2006). At the moment, the class supports only GWCS-based WCS. This is something we can address in a future update.

[codecov]

Codecov Report

❌ Patch coverage is 94.19355% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.99%. Comparing base (61a5d4a) to head (c4eadb4).

Files with missing lines	Patch %	Lines
specreduce/wavecal1d.py	94.18%	27 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #265      +/-   ##
==========================================
+ Coverage   87.11%   88.99%   +1.87%     
==========================================
  Files          15       16       +1     
  Lines        1281     1745     +464     
==========================================
+ Hits         1116     1553     +437     
- Misses        165      192      +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[hpparvi]

Ok, looks like all the builds are failing because of a matplotlib dependency that I added. I'll need to make matplotlib and the plotting functionality optional.

[hpparvi]

Docs build works with Jupyter Notebook examples. Take a look at https://specreduce--265.org.readthedocs.build/en/265/wavelength_calibration/wavelength_calibration.html

[cshanahan1]

I gave this a first pass and left a few comments. Notebook outputs need to be cleared, and i think it might make sense to have a data subdirectory rather than putting these files in docs

[cshanahan1]

should be from specreduce import WavelengthCalibration1D if you want to import it in init like wavelengh_calibration, otherwise just a typo and should be specreduce.lswavecal1d import WavelengthSolution1D

[hpparvi]

Thanks for catching this @cshanahan1. This notebook was outdated, and I shouldn't have included it in the PR in the first place.

[cshanahan1]

need to provide the path ../docs/wavelength_calibration/wavelength_calibration/, and also it looks like the file in that directory is different shane_kast_blue_600_4310_d55.fits so either change the filename to that one or add the correct file.

[cshanahan1]

This class doesn't exist, if this should replaced with WavelengthCalibration1D and wlbounds needs to be line_list_bounds and ref_pixel should be supplied. Also remove %%time from the top of this cell. I got a traceback when I tried this replacement with 'ref_pix', so i'll wait to continue reviewing the rest of this notebook

[tepickering]

The only (somewhat) significant feature currently missing is support for generating standard spectroscopic -GRI and -GRA WCSs as described in Greisen et al. (2006). At the moment, the class supports only GWCS-based WCS. This is something we can address in a future update.

i need to sit down and give this PR a proper review, but wanted to make a quick comment on this. i think GWCS should be the primary focus because it supports everything we will need to do while the FITS standard only really applies to the 1D case. the FITS standard for 2D spectra was never fully adopted or fleshed out and what exists is insufficient for our needs. as such, i think it's fine to only support GWCS for now. backwards compatibility for cases where it's possible can easily be added later. GWCS already provides a to_fits method that generates a -TAB based WCS for spectral coordinates. that may be fine for most users.

[tepickering]

weird that those Pillow deprecation warnings are only happening under windows. i tracked down the offending function call and it looks like the fix was made in 3.10.x to remove the to-be-deprecated mode parameter. one option would be to just bump the matplotlib requirement to 3.10. another would be to catch that warning and accept it for now. i'd lean towards bumping the requirement and being done with it...

[hpparvi]

Thanks @tepickering! I've removed the 2D tilt correction code and tutorials from this PR, and I have continued revising the code and docs based on new datasets I've tested the code with. Feel free to take a deeper look.

The only thing I may still want to change is the name of the fit_global method. This method is intended for non-interactive workflows (automatic pipelines with relatively stable instruments), while the fit_lines is meant for interactive workflows. The main difference between the two is that fit_lines requires a set of user-provided matched lines in pixel and wavelength space, and fit_global doesn't. The "global" in the name refers to the method's use of global optimization for calculating the wavelength solution. However, the name doesn't describe what the method does or explain how it differs from fit_lines. Does anyone have good alternative names for the method?

[hpparvi]

Hi @tepickering, @cshanahan1 and @gibsongreen. Let me know if any of you have the time to take a look at this PR in the near future. I’ve removed the tutorials from the PR (they’ll go under astropy-learn instead) but have added two instrument-specific examples, available here:

https://github.com/hpparvi/NIRSpec_wavecal_example

https://github.com/hpparvi/specreduce_example_lhires

It would be great to get this PR merged so we can move on to the 2D tilt correction, documentation facelift, and uncertainty handling. I also suggest we make a new release after merging this, as the 2D tilt correction will likely require significant reviewer effort.