jwst
jwst copied to clipboard
JP-3470: Add ifu covariance scaling to extract1d
Resolves JP-3470 by allowing for a parameter to be passed into extract1d accounting for IFU cube covariance.
Closes #8083
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)
- [x] 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 13.33333% with 13 lines in your changes missing coverage. Please review.
Project coverage is 57.98%. Comparing base (
781e0e0) to head (85eefe9). Report is 294 commits behind head on master.
| Files | Patch % | Lines |
|---|---|---|
| jwst/extract_1d/ifu.py | 13.33% | 13 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #8457 +/- ##
==========================================
+ Coverage 57.93% 57.98% +0.05%
==========================================
Files 387 387
Lines 38839 38826 -13
==========================================
+ Hits 22502 22514 +12
+ Misses 16337 16312 -25
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
regtest run started at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1424
CI tests are failing with:
self = <LogRecord: stpipe, 20, /home/runner/work/jwst/jwst/jwst/extract_1d/ifu.py, 248, "Applying ERR covariance prefactor of %g">
def getMessage(self):
"""
Return the message for this LogRecord.
Return the message for this LogRecord after merging any user-supplied
arguments with the message.
"""
msg = str(self.msg)
if self.args:
> msg = msg % self.args
E TypeError: must be real number, not NoneType
CI tests are failing with:
self = <LogRecord: stpipe, 20, /home/runner/work/jwst/jwst/jwst/extract_1d/ifu.py, 248, "Applying ERR covariance prefactor of %g"> def getMessage(self): """ Return the message for this LogRecord. Return the message for this LogRecord after merging any user-supplied arguments with the message. """ msg = str(self.msg) if self.args: > msg = msg % self.args E TypeError: must be real number, not NoneType
Hm, I thought the default argument of 1.0 in the function call would be enough to deal with this, but I guess not. Added a check for value defined prior to doing anything with it.
I'd like to check the issue with the parameter default value not being picked up before we merge this, to verify it's not a larger issue.
Started another regtest run at https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1454
And the CI tests seem to suddenly be working OK. Not sure why ...
Latest regtest run is clean. Only failures are unrelated (ref file updates).
So this is ready to merge as soon as @nden is OK with the earlier issue with function arguments.
I tracked the problem with the arguments to a test
jwst/extract_1d/tests/test_ifu_image_ref.py: test_ifu_3d
which calls extract.do_extract_1d directly without passing a value for the parameter and so it uses the default in the do_extract1d function which is None.
So passing of parameters works as expected but if extract.do_extract_1d is to be called on its own I suggest we set a default value in the signature as well. Also remove the condition here.
Alternatively, instead of removing the condition here, it could be modified to only trigger when ifu_covar_scale is not equal to 1.0. That way you avoid having the unnecessary message about it being applied when it really isn't having any effect on the data.
I've changed that None default to 1.0, and changed the conditional to trigger when the value is not 1.0
Final updates look good to me. Merging.