JP-1885 Changes for when there are only three and four good groups for jump detection #5915
Conversation
In preperation for the code walkthrough I updated the comments and renamed a few variables to be more descriptive. I also updated a couple of the test cases.
get up to date
get up to date on 12/31/2018
Get up to date
Get my repo up to date
get repo up to date
Initial Changes to add flagging neihbors of CRs
Update Master to Current ST Master
Partial changes on wrong branch removed.
Try to fix conflict
Update to Latest Mater 2/21/2020 #2
Get Master up to date 3/6
There was a problem hiding this comment.
This latest set of updates seem fine. I do have a few mostly minor editorial comments about two_point_difference.py :
-
How are pixels with only a single good group handled within find_crs( ) ? If such pixels are filtered out before find_crs() is called perhaps clarifying that in the docs would help.
-
Some variables have long names, such as 'three_diff_rejection_threshold' (30 characters ). While very descriptive, readability of statements having several such variables could be improved, with shorter names and docstrings for example.
-
'ndiffs- number_CRs_found - pixel_sat_groups' is used 4 times within a while loop; perhaps use a single variable instead to ease readability.
-
In :
def get_clipped_median_array(num_differences, diffs_to_ignore, differences, sorted_index)
" This routine will return the clipped median for the input array."
What is the 'input array' ? I can't tell, based on the names of the arguments. Documentation (docstrings) of the input arguments (including types) would help clarify this.
Update to April 16 2021 master
Pep8 and variable name updates
…ups-for-jump-detection' of https://github.com/mwregan2/jwst into Changes-for-when-there-are-only-three-and-four-good-groups-for-jump-detection
Update to current master
Fix change in parameter name to find_median routines, comment XFAIL tests and rework and XFAIL to a normal expected pass version.
Fixed change to calling parameter name.
removed unused variable
Changed test for minimum number of groups to be 3 instead of 5.
…ups-for-jump-detection' of https://github.com/mwregan2/jwst into Changes-for-when-there-are-only-three-and-four-good-groups-for-jump-detection
Add jump step changes
Remove redundant test.
Update comments to explain parameters better. Also, create a new local variable for tracking the index of the largest difference to use.
Remove two tests that had no asserts and didn't test anything.
Fixed white space issues
Update read the docs to match changes for three and four group integrations.
|
I think I've addressed all of David and Howard's comments. I've also updated read-the-docs and the inline comments to match the current version. I've fixed all the PEP8 problems and fixed the bugs I introduced fixing the PEP8 problems too. |
Moved jump changes to be in 1.1.1 and added PR to description
mwregan2
deleted the
Changes-for-when-there-are-only-three-and-four-good-groups-for-jump-detection
branch
Closes #5688
Resolves JP-1885 (partially)
Description
This PR addresses the first half of JP-1885. It adds the capability to detect jumps in three and four frame integrations. The baseline version only operates on integrations with at least five frame.
Checklist