-
Notifications
You must be signed in to change notification settings - Fork 169
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
JP-3087: Fix zeroing of NaNs in straylight #7455
Conversation
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.
Code looks fine. Would it be way too difficult to update the underlying model code to be able to cope with NaNs, as opposed to replacing them before sending down?
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
Codecov ReportBase: 77.90% // Head: 77.90% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #7455 +/- ##
=======================================
Coverage 77.90% 77.90%
=======================================
Files 455 455
Lines 36975 36977 +2
=======================================
+ Hits 28806 28808 +2
Misses 8169 8169
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
Looks fine to me.
Regression test run started at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/573/ |
regtest run showed 1 unrelated failure, due to recent changes in stcal/ramp_fitting. So it's good. |
This PR fixes a new issue where the MRS straylight routine converts NaNs to zeros. Previously this was a safety catch in case any NaNs were present that were not meant to be there, but now that the rate file is meant to contain NaNs this behaviour is undesirable. This PR modifies straylight to only do the NaN-to-zero conversion in a local copy of the input used for the straylight computation itself.
Resolves JP-3087
Checklist for maintainers
CHANGES.rst
within the relevant release sectionHow to run regression tests on a PR