JP-1737: Propagate error through Extract1D step #6014
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6014 +/- ##
==========================================
- Coverage 76.99% 75.31% -1.69%
==========================================
Files 401 401
Lines 34063 35232 +1169
==========================================
+ Hits 26228 26536 +308
- Misses 7835 8696 +861
*This pull request uses carry forward flags. Click here to find out more.
Continue to review full report at Codecov.
|
jwst/extract_1d/extract.py
Outdated
| ) -> Tuple[ | ||
| float, float, np.ndarray, np.ndarray, np.ndarray, np.ndarray, np.ndarray | ||
| ]: | ||
| def extract(self, data, |
There was a problem hiding this comment.
data is missing an np.ndarray type declaration
There was a problem hiding this comment.
Looks like this still needs fixing (add np.ndarray declaration for data)
jwst/extract_1d/extract1d.py
Outdated
| @@ -81,9 +93,27 @@ def extract1d(image, lambdas, disp_range, | |||
| countrate : ndarray, 1-D, float64 | |||
| The extracted spectrum in units of counts / s. | |||
There was a problem hiding this comment.
Really should finally update these comments and maybe even the name of the countrate object. The references to countrate and counts/s are based on the old days when the extraction was done on data that had not been flux calibrated (flux calibration was applied after extraction). Now the normal case is to be working on flux calibrated data and hence the units are MJy/sr or Jy. Only if the user has skipped the photom step in earlier processing will the data ever be in count rate units.
There was a problem hiding this comment.
I was curious about that - it didn't seem to jive with the rest of the documentation around these files. I'll update it.
There was a problem hiding this comment.
Still need to update these descriptions for the param list. I know it doesn't affect operation of the code, but let's not leave these lingering here for the future.
…-error-x1d merge into master for up-to-date regtest results
|
Lingering regression test issue has been resolved - the combine1d difference was caused by a missing meta.observation.bkgdtarg = 'EXTENDED' - user error in updating the new datamodel with only='PRIMARY'... |
jwst/extract_1d/extract.py
Outdated
| ) -> Tuple[ | ||
| float, float, np.ndarray, np.ndarray, np.ndarray, np.ndarray, np.ndarray | ||
| ]: | ||
| def extract(self, data, |
There was a problem hiding this comment.
Looks like this still needs fixing (add np.ndarray declaration for data)
jwst/extract_1d/extract1d.py
Outdated
| @@ -81,9 +93,27 @@ def extract1d(image, lambdas, disp_range, | |||
| countrate : ndarray, 1-D, float64 | |||
| The extracted spectrum in units of counts / s. | |||
There was a problem hiding this comment.
Still need to update these descriptions for the param list. I know it doesn't affect operation of the code, but let's not leave these lingering here for the future.
Cleaned these up - I did confirm that AttributeError is triggered using the test_miri_mrs_extract1d regtest. |
…stro/jwst into jp-1737-propagate-error-x1d hopefully this fixes the changes merge
Closes #5373
Resolves JP-1737
Description
This PR propagates error/variance arrays through the various branches of Extract1D. Some existing functions were modified out of convenience when handling many new arrays. A first pass on documentation has been completed.
Working on regression test results now.
Checklist