-
Notifications
You must be signed in to change notification settings - Fork 28
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
add class aliases for steps #1509
Conversation
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.
Great. Thanks Brett. Can you remind me if this has regression test impacts? I agree it's good to have the class aliases.
I expect this won't change regression tests (yet). There is still an issue where stpipe uses a different pattern for setting This PR does mean that someone could call:
But this PR doesn't update the docs to encourage/discourage that (and I think opening and merging #1476 makes sense to do before that). |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1509 +/- ##
==========================================
+ Coverage 76.21% 76.25% +0.03%
==========================================
Files 115 115
Lines 7627 7639 +12
==========================================
+ Hits 5813 5825 +12
Misses 1814 1814 ☔ View full report in Codecov by Sentry. |
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.
Thanks, Brett!
As discussed in spacetelescope/roman_datamodels#343 stpipe expects the
class_alias
of a step to match themeta.cal_step.<class_alias>
attribute assigned when a step is skipped.https://github.com/spacetelescope/stpipe/blob/677f6a43b7c101f490a5d923a653d20c7fbe493b/src/stpipe/step.py#L485
This PR updates all steps in romancal to have a
class_alias
that matches the string used in thel2_cal_step
schema:https://github.com/spacetelescope/rad/blob/main/src/rad/resources/schemas/l2_cal_step-1.0.0.yaml
Regression tests all pass: https://github.com/spacetelescope/RegressionTests/actions/runs/11801390936
Tasks
24Q4_B15
(use the latest build if not sure)no-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see below for change types)docs/
pageokify_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#>.associations.rst
changes/<PR#>.scripts.rst
changes/<PR#>.mosaic_pipeline.rst
changes/<PR#>.patch_match.rst
steps
changes/<PR#>.dq_init.rst
changes/<PR#>.saturation.rst
changes/<PR#>.refpix.rst
changes/<PR#>.linearity.rst
changes/<PR#>.dark_current.rst
changes/<PR#>.jump_detection.rst
changes/<PR#>.ramp_fitting.rst
changes/<PR#>.assign_wcs.rst
changes/<PR#>.flatfield.rst
changes/<PR#>.photom.rst
changes/<PR#>.flux.rst
changes/<PR#>.source_detection.rst
changes/<PR#>.tweakreg.rst
changes/<PR#>.skymatch.rst
changes/<PR#>.outlier_detection.rst
changes/<PR#>.resample.rst
changes/<PR#>.source_catalog.rst