-
Notifications
You must be signed in to change notification settings - Fork 180
JP-3941: Refactor white_light to get correct time stamps and spectral orders for NIRISS #9431
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
|
regtest round started here: https://github.com/spacetelescope/RegressionTests/actions/runs/14802258787 Only one failure, as expected, in |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9431 +/- ##
==========================================
+ Coverage 77.92% 77.93% +0.01%
==========================================
Files 362 362
Lines 36283 36252 -31
==========================================
- Hits 28272 28253 -19
+ Misses 8011 7999 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
melanieclarke
left a comment
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.
The output format looks good to me but we'll wait for INS review.
This will need integration with #9430 - this PR should go in first, then I will update for the new TSO format.
melanieclarke
left a comment
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.
A couple minor comments from working with these changes to update them for #9430
melanieclarke
left a comment
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, but we should make sure INS is happy with the format before merging.
|
Sounds good. It seems like they are generally unhappy with the step output. My plan is to just write a comment on the JIRA ticket asking if this is at least an improvement, and to start a separate discussion about the actual table contents... does that sound good? |
Perfect. Thanks! |
drlaw1558
left a comment
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.
LG2M
Resolves JP-3941
This PR refactors the white light step to return sensible results for NIRISS data.
On main, and e.g. in the current truth files of our regtests (see e.g.
truth/test_niriss_soss_stages/jw01091-o002_t001_niriss_clear-gr700xd-substrip256_whtlt.ecsv, the_whtlt.ecsvfiles have two major issues:This PR fixes both of those issues - compare the file mentioned above with the one attached to the JIRA ticket, which came from the same input association.
The control logic of the step is also simplified.
Outputs are unaffected for NIRCam tsgrism and MIRI LRS slitless TSO data.
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/pageokify_regteststo update the truth files