-
Notifications
You must be signed in to change notification settings - Fork 177
JP-3905 add option to save shower model in straylight step for MIRI MRS data #9378
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-3905 add option to save shower model in straylight step for MIRI MRS data #9378
Conversation
Running the regression tests and I will add a test to compare the shower model |
a6c0a93
to
92064f3
Compare
Oh I need to probably update the docs to add this new parameter. I will do that. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9378 +/- ##
==========================================
+ Coverage 75.37% 75.58% +0.21%
==========================================
Files 368 368
Lines 36885 36898 +13
==========================================
+ Hits 27802 27890 +88
+ Misses 9083 9008 -75 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
92064f3
to
73f7985
Compare
running regression tests with new test. |
Regression tests passed: |
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.
Testing locally, it looks like the file is saved as expected. Thanks for updating the docs and adding the new test as well!
It would be nice to add a unit test for the new output, as well as the regression test, to make sure it gets saved in the right location when output_dir is specified.
A few more comments and requests below.
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.
Checking in on your updates... A couple more clean up items remaining, and another follow up request for the regtest.
Looks like the unit test is still not working because: "Straylight correction not defined for datatype CubeModel". It will need a different synthetic input than the other unit test.
3951f3b
to
b12e4d0
Compare
@ re-running regression tests |
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.
I'll rerun regtests one more time at the same link, but then I think this should be good to go.
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.
All good now!
Resolves JP-3905
Closes #9240
This PR adds an option to the straylight step to save the shower model when also running the clean_shower option.
A new regression test running straylight with clean_showers and save_shower_model = True.
New regression Truth files were added to the regression test suite.
Tasks
Build 12.0
(use the latest build if not sure)no-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see changelog readme for instructions)docs/
pageRegression tests passed:
https://github.com/spacetelescope/RegressionTests/actions/runs/14474571962
okify_regtests
to update the truth files