-
Notifications
You must be signed in to change notification settings - Fork 174
JP-3547: Fix MIRI array sizing for multiple inputs and pixel ratio != 1 #8727
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
Conversation
Regression tests here: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8727 +/- ##
=======================================
Coverage 60.55% 60.55%
=======================================
Files 372 372
Lines 38371 38371
=======================================
Hits 23235 23235
Misses 15136 15136 ☔ View full report in Codecov by Sentry. |
Regtest failures all look unrelated - there are no changes to any spectral resampling products. |
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 looks great to me, just had two small questions about the unit test.
LG2M: Fixes the issue with LRS slit data, and results look as expected in a test case of spec3-combined dithered data. |
@tapastro - can you please review for the maintainers? |
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.
LGTM - will wait for CI then merge.
Resolves JP-3547
Fixes an issue discovered during testing for #8596.
For MIRI, when there are multiple input files and pixel_scale_ratio is set to something other than 1.0, the output array size was being computed incorrectly: the pixel_scale_ratio was applied twice. This PR fixes that issue and adds a unit test for the multiple input case.
Checklist for PR authors (skip items if you don't have permissions or they are not applicable)
CHANGES.rst
within the relevant release sectionupdated relevant documentationHow to run regression tests on a PR