jwst
jwst copied to clipboard
WIP: JP-3463: Add average_dark_current handling to dark_current step
Resolves JP-3463
Closes #8071
This PR adds pipeline handling of new datamodel quantities for the average dark current. This can be specified either by the user, as a scalar quantity, or in the dark reference file, as a scalar quantity or a 2D array matching the dark reference file size. If the user provides a value, it will override any dark reference value. If a user wishes to provide a 2D array, it will be left to them to generate a custom dark.
This quantity has been/wiill be added to the RampModel in https://github.com/spacetelescope/stdatamodels/pull/265; it will be stored in the RampModel for use in the ramp_fitting step.
NOTE: This PR is labelled WIP until the stdatamodels PR linked above is merged.
Checklist for maintainers
- [x] added entry in
CHANGES.rstwithin the relevant release section - [ ] updated or added relevant tests
- [x] updated relevant documentation
- [x] added relevant milestone
- [x] added relevant label(s)
- [ ] ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR
- [ ] Make sure the JIRA ticket is resolved properly
Codecov Report
Attention: Patch coverage is 70.00000% with 3 lines in your changes are missing coverage. Please review.
Project coverage is 53.15%. Comparing base (
4cc0ac1) to head (fd5406f). Report is 22 commits behind head on master.
| Files | Patch % | Lines |
|---|---|---|
| jwst/dark_current/dark_current_step.py | 70.00% | 3 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #8302 +/- ##
===========================================
- Coverage 75.15% 53.15% -22.01%
===========================================
Files 470 389 -81
Lines 38604 38442 -162
===========================================
- Hits 29014 20434 -8580
- Misses 9590 18008 +8418
| Flag | Coverage Δ | |
|---|---|---|
| nightly | ? |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@tapastro If you can address my couple of review questions I'd be willing to approve. Unless you think it needs more work.
I just need to spend some time determining if that test provides any useful coverage, or if I should get another regression test parametrization in that sets the dark current to ensure it's doing someting. I'll work on better testing and get back to you next week.
Regression tests here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1301/
I added a test to ensure data is tested with a non-zero average dark current specified. This test can be removed if/when darks are delivered with non-zero values.
The 1301 regtest run shows only the expected differences in products created during Detector1 processing (before ramp fitting), which now contain either the extra "AVDRKCUR" image extension or the new "AVDRKCUR" keyword. So that looks good.
I have to say I'm a little sad the message for this commit didn't make it into main due to the squash :) https://github.com/spacetelescope/jwst/pull/8302/commits/fd5406fedb7b56617790a268f67c4d43cb57a572 It describes how I often feel.