Jump masked groups counting #317
Conversation
Add unit tests
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #317 +/- ##
==========================================
+ Coverage 83.79% 86.44% +2.65%
==========================================
Files 35 49 +14
Lines 6998 8964 +1966
==========================================
+ Hits 5864 7749 +1885
- Misses 1134 1215 +81 ☔ View full report in Codecov by Sentry. |
| for grp in range(dat.shape[1]): | ||
| if np.all(np.bitwise_and(gdq[integ, grp, :, :], dnu_flag)): | ||
| num_flagged_grps += 1 | ||
| if num_flagged_grps > max_flagged_grps: |
There was a problem hiding this comment.
The adjustment of max_flagged_grps can be done after the grp loop. Doing this check for every loop over grp is inefficient.
| dummy = np.zeros((dataa.shape[1] - 1, dataa.shape[2], dataa.shape[3]), | ||
| dtype=np.float32) | ||
| return gdq, row_below_gdq, row_above_gdq, 0, dummy | ||
| dummy = np.zeros((dataa.shape[1] - 1, dataa.shape[2], dataa.shape[3]), dtype=np.float32) |
There was a problem hiding this comment.
Descriptive variable names for the dataa shape is defined here:
stcal/src/stcal/jump/twopoint_difference.py
Line 139 in fb6bfea
Use these variable names, instead of shape[k].
| dummy = np.zeros((dataa.shape[1] - 1, dataa.shape[2], dataa.shape[3]), dtype=np.float32) | ||
| return gdq, row_below_gdq, row_above_gdq, -99, dummy | ||
| # | ||
| # if ((only_use_ints and nints < minimum_sigclip_groups) # sigclip across ints with not enough ints |
There was a problem hiding this comment.
These comments should be deleted.
| @@ -239,7 +259,7 @@ def find_crs( | |||
| # use mask on data, so the results will have sat/donotuse groups masked | |||
| first_diffs = np.diff(dat, axis=1) | |||
|
|
|||
| if total_usable_diffs >= min_diffs_single_pass: | |||
| if min_usable_diffs >= min_diffs_single_pass: # There are enough diffs in all ints to look for more than one jump | |||
There was a problem hiding this comment.
Put comment on the next line.
| @@ -21,6 +21,98 @@ def _cube(ngroups, nints=1, nrows=204, ncols=204, readnoise=10): | |||
|
|
|||
| return _cube | |||
|
|
|||
| def test_sigclip_not_enough_groups(setup_cube): | |||
| ngroups = 10 | |||
| nints = 9 | |||
There was a problem hiding this comment.
Variables nints, nrows, and ncols are not used, so should be deleted.
|
|
||
| def test_sigclip_not_enough_groups(setup_cube): | ||
| ngroups = 5 | ||
| nints = 9 |
There was a problem hiding this comment.
Variables nints, nrows, and ncols are not used, so should be deleted.
|
|
||
| def test_sigclip_not_enough_groups(setup_cube): | ||
| ngroups = 12 | ||
| nints = 9 |
There was a problem hiding this comment.
Variables nints, nrows, and ncols are not used, so should be deleted.
Resolves JP-3793
This PR addresses a problem in the two point difference code where it determines if there are enough groups for the code to run. The logic was not correct and has now been corrected.
Tasks
docs/pageno-changelog-entry-needed)changes/:echo "changed something" > changes/<PR#>.<changetype>.rst(see below for change types)"git+https://github.com/<fork>/stcal@<branch>")jwstregression testromancalregression testnews fragment change types...
changes/<PR#>.apichange.rst: change to public APIchanges/<PR#>.bugfix.rst: fixes an issuechanges/<PR#>.general.rst: infrastructure or miscellaneous change