Skip to content

1D Wavelength Calibration and 2D tilt correction #265

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 73 commits into
base: main
Choose a base branch
from

Conversation

hpparvi
Copy link
Contributor

@hpparvi hpparvi commented Apr 24, 2025

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), and a new tilt correction class for 2D tilt correction. It also includes documentation demonstrating how to use the class for both interactive and automated wavelength calibration workflows.

The core functionality is ready for review. I still need to tidy up some of the docstrings and ensure the ReadTheDocs configuration correctly supports Jupyter notebooks, but otherwise everything is in place. Feel free to test it out and let me know if you run into any issues.

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.

Copy link

codecov bot commented Apr 24, 2025

Codecov Report

Attention: Patch coverage is 97.17813% with 16 lines in your changes missing coverage. Please review.

Project coverage is 90.19%. Comparing base (1220a73) to head (1c40831).

Files with missing lines Patch % Lines
specreduce/wavecal1d.py 97.58% 9 Missing ⚠️
specreduce/tilt_correction.py 96.41% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #265      +/-   ##
==========================================
+ Coverage   87.64%   90.19%   +2.54%     
==========================================
  Files          15       17       +2     
  Lines        1279     1846     +567     
==========================================
+ Hits         1121     1665     +544     
- Misses        158      181      +23     

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

hpparvi commented Apr 24, 2025

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

hpparvi commented Apr 24, 2025

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

@hpparvi hpparvi requested review from camipacifici and kecnry April 24, 2025 19:06
@hpparvi hpparvi added this to the v1.6 milestone Apr 24, 2025
@hpparvi hpparvi changed the title 1D Wavelength Calibration 1D Wavelength Calibration and 2D rectification May 15, 2025
@hpparvi hpparvi changed the title 1D Wavelength Calibration and 2D rectification 1D Wavelength Calibration and 2D tilt correction May 20, 2025
Copy link
Contributor

@cshanahan1 cshanahan1 left a comment

Choose a reason for hiding this comment

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

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

"from matplotlib.pyplot import setp, subplots, close, rc\n",
"from specutils import Spectrum1D\n",
"\n",
"from specreduce.lswavecal1d import WavelengthSolution1D\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

"metadata": {},
"outputs": [],
"source": [
"flux = getdata('shane_kast_blue_600_4310_d55.fits', 1).astype('d')\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

],
"source": [
"%%time\n",
"ws = WavelengthSolution1D(arc_spectra=arc_spectrum, line_lists=[['CdI', 'HgI', 'HeI']], wlbounds=(3200, 5700))\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

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

hpparvi and others added 19 commits June 17, 2025 13:18
… a sum of nearest-line distances between a theoretical line list and a transformed observed line list.
Renamed `wavecal.py` to `lswavecal1d.py` and refactored the `WavelengthSolution1D` class for improved clarity and functionality. Added new methods like `refine_fit`, `pix_to_wav`, and `wav_to_pix`, and modified existing ones to enhance flexibility and maintainability.
Introduce `WavelengthSolution2D` class for 2D wavelength calibration, extending functionality from 1D calibration.
… type hints, documentation, and method structure. Added new methods for calculating inverse transformations and derivatives. Enhanced plotting functionality with additional parameters and refined docstrings for clarity.
… `plim` parameter for pixel range control in plotting.
…, and modularity, while resolving associated issues. Enhanced methods for handling pixel-to-wavelength mappings, line-matching processes, and plotting capabilities. Added new functions and properties for better control over data and fit refinement.
…ns with setters to ensure proper masking and validation. Added `plot_lines_pix` and `plot_lines_wav` methods for enhanced modularity in plotting observed and theoretical lines.
hpparvi added 26 commits June 17, 2025 13:19
…on't break backwards compatibility (quite yet).
…, and functionality. Added detailed docstrings, refined methods for arc line identification, transformation fitting, and matching lines. Improved internal handling of parameters and optimized logic.
…onsistency and improved usability. Updated variable names and docstrings for better clarity and alignment with the code's functionality. Simplified return behavior in `match_lines` to support optional concatenation.
…xibility in 2D spectrum rectification. Updated methods to use these parameters, ensuring proper alignment and control over mask handling. Also improved documentation and refactored variable naming.
…uns, but must be improved to ensure that the results are correct.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants