Skip to content
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

First Pass WCS extract region bound fix #882

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

Conversation

duytnguyendtn
Copy link
Contributor

Addresses #869
@PatrickOgle identified a bug in extract_regions where the WCS wavelength reference value (CRVAL0) wasn't being updated to the new bounds of the extracted region. Implemented Patrick's solution in this PR. Still need to figure out what the appropriate solution would be for GWCS though...

@duytnguyendtn
Copy link
Contributor Author

Poked around in SpectralGWCS, other than adjusting the size of the bounding box, I'm not sure if I can see a place where an equivalent slice would be appropriate, or even possible. I'm not sure, and the ticket #869 doesn't suggest, this problem exists with SpectralGWCS defined WCS's, so I'm going to mark this ready for review and request someone who knows better than I do clarify!

@duytnguyendtn duytnguyendtn marked this pull request as ready for review October 15, 2021 21:01
@PatrickOgle
Copy link
Contributor

@duytnguyendtn I am not familiar with GWCS and have not tried to use spectral_slab on it. We can either:

  1. Test this PR on a Spectrum1D with GWCS
  2. ...OR file a ticket for GWCS later if there is problem
    ...your choice.

@pllim
Copy link
Member

pllim commented Dec 23, 2022

As discussed, if we want to revisit, the solution should adhere to APE 14 high-level API if possible. That way, it would naturally work for both FITS WCS and GWCS.

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.

None yet

3 participants