-
-
Notifications
You must be signed in to change notification settings - Fork 133
Update WCS when shifting spectrum and fix bugs with arithmetic and redshift #1287
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1287 +/- ##
==========================================
- Coverage 86.98% 86.79% -0.20%
==========================================
Files 63 63
Lines 4857 4870 +13
==========================================
+ Hits 4225 4227 +2
- Misses 632 643 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Codestyle
specutils/spectra/spectrum.py
Outdated
| wcs_spectral_index = self.wcs.naxis - self.spectral_axis_index | ||
| h = self.wcs.to_header() | ||
| z_factor = (1 + redshift) / (1 + old_redshift) | ||
| h[f'CRVAL_{wcs_spectral_index}'] *= z_factor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this relies on ctype being wavelength
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call - the factor would just be the inverse of this for frequency, right? Then of course there's energy and wavenumber, but perhaps I don't worry about those for now 😅 .
Edit: wait, those two are also the inverse I think, not that complicated.
|
Test failures are unrelated timeouts. |
Fixes #1268, and additionally improves the behavior of the
shift_spectrum_tomethod.Previously, the spectral axis was updated when applying a redshift with
shift_spectrum_to, but the original WCS was kept on theSpectrum, which led to some annoying behavior and workarounds (and was potentially confusing to the user). Now the WCS will be updated along with the spectral axis - in the case of a FITS WCS, the relevant header values will be updated in place, while a GWCS will be replaced with a new lookup table GWCS. In the latter case, the original WCS will be stored in an_original_wcsattribute so no information is lost (e.g., spatial coordinates for a cube). This obviously isn't an ideal solution for the GWCS case but I hope it suffices while I investigate better ways to handle the GWCS case.