-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JP- 3778 Allow the RSCD correction to work on segmented TSO data #8946
JP- 3778 Allow the RSCD correction to work on segmented TSO data #8946
Conversation
@karllark flagged you to review this since you originally wrote much of the RSCD step. |
9d65c82
to
a87d763
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8946 +/- ##
=======================================
Coverage 64.53% 64.54%
=======================================
Files 375 375
Lines 38728 38737 +9
=======================================
+ Hits 24993 25002 +9
Misses 13735 13735 ☔ View full report in Codecov by Sentry. |
c723d09
to
6b5cd45
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. One minor comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, and appears to be working as expected.
@jemorrison - I know you were thinking about adding a regression test for this case, but I don't see one here. Did you decide not to, or did it not get pushed?
@melanieclarke Somehow the regtest got left out of PR. I push the new test up |
daa5bf6
to
41a61a9
Compare
41a61a9
to
a9ab10a
Compare
@melanieclarke This is ready for final review |
Looks good now, and seems to be working in local tests. Did you re-run the regression tests? |
Never mind, I see from the link above that you did re-run them yesterday, and results were clean. I'll go ahead and approve and get this merged. Thanks for working it through! |
@melanieclarke Yes the link to the regression tests (above) is after I okifyed the tests yesterday. |
Resolves JP-3778
Closes #8878
This PR addresses allows the RSCD step to work on segmented data. In segmented data the first integration in the data may not be the first integration in the full exposure. If this is the case, we want to flag initial groups in the integration as DO_NOT_USE.
Tasks
Build 11.3
(use the latest build if not sure)no-changelog-entry-needed
)write news fragment(s) in
changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see below for change types)update or add relevant tests
update relevant docstrings and / or
docs/
pagestart a regression test and include a link to the running job ([click here for instructions]
Results: (all tests passed) https://github.com/spacetelescope/RegressionTests/actions/runs/12034294166
(https://github.com/spacetelescope/RegressionTests/blob/main/docs/running_regression_tests.md))
okify_regtests
to update the truth filesnews fragment change types...
changes/<PR#>.general.rst
: infrastructure or miscellaneous changechanges/<PR#>.docs.rst
changes/<PR#>.stpipe.rst
changes/<PR#>.datamodels.rst
changes/<PR#>.scripts.rst
changes/<PR#>.fits_generator.rst
changes/<PR#>.set_telescope_pointing.rst
changes/<PR#>.pipeline.rst
stage 1
changes/<PR#>.group_scale.rst
changes/<PR#>.dq_init.rst
changes/<PR#>.emicorr.rst
changes/<PR#>.saturation.rst
changes/<PR#>.ipc.rst
changes/<PR#>.firstframe.rst
changes/<PR#>.lastframe.rst
changes/<PR#>.reset.rst
changes/<PR#>.superbias.rst
changes/<PR#>.refpix.rst
changes/<PR#>.linearity.rst
changes/<PR#>.rscd.rst
changes/<PR#>.persistence.rst
changes/<PR#>.dark_current.rst
changes/<PR#>.charge_migration.rst
changes/<PR#>.jump.rst
changes/<PR#>.clean_flicker_noise.rst
changes/<PR#>.ramp_fitting.rst
changes/<PR#>.gain_scale.rst
stage 2
changes/<PR#>.assign_wcs.rst
changes/<PR#>.badpix_selfcal.rst
changes/<PR#>.msaflagopen.rst
changes/<PR#>.nsclean.rst
changes/<PR#>.imprint.rst
changes/<PR#>.background.rst
changes/<PR#>.extract_2d.rst
changes/<PR#>.master_background.rst
changes/<PR#>.wavecorr.rst
changes/<PR#>.srctype.rst
changes/<PR#>.straylight.rst
changes/<PR#>.wfss_contam.rst
changes/<PR#>.flatfield.rst
changes/<PR#>.fringe.rst
changes/<PR#>.pathloss.rst
changes/<PR#>.barshadow.rst
changes/<PR#>.photom.rst
changes/<PR#>.pixel_replace.rst
changes/<PR#>.resample_spec.rst
changes/<PR#>.residual_fringe.rst
changes/<PR#>.cube_build.rst
changes/<PR#>.extract_1d.rst
changes/<PR#>.resample.rst
stage 3
changes/<PR#>.assign_mtwcs.rst
changes/<PR#>.mrs_imatch.rst
changes/<PR#>.tweakreg.rst
changes/<PR#>.skymatch.rst
changes/<PR#>.exp_to_source.rst
changes/<PR#>.outlier_detection.rst
changes/<PR#>.tso_photometry.rst
changes/<PR#>.stack_refs.rst
changes/<PR#>.align_refs.rst
changes/<PR#>.klip.rst
changes/<PR#>.spectral_leak.rst
changes/<PR#>.source_catalog.rst
changes/<PR#>.combine_1d.rst
changes/<PR#>.ami.rst
other
changes/<PR#>.wfs_combine.rst
changes/<PR#>.white_light.rst
changes/<PR#>.cube_skymatch.rst
changes/<PR#>.engdb_tools.rst
changes/<PR#>.guider_cds.rst