Skip to content
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

DM-32694: Split AP pipeline into ApPipeWithFakes #159

Merged
merged 6 commits into from May 17, 2022
Merged

Conversation

kfindeisen
Copy link
Member

This PR refactors the ApVerifyWithFakes pipeline to depend on ApPipeWithFakes (itself reworked on lsst/ap_pipe#106) instead of ApVerify. This change improves consistency with the "base" AP pipeline and avoids some of the contract conflicts that made the previous ApVerifyWithFakes more complicated and more brittle than it needed to be.

Explicitly defining a parameter for each possible dataset was too much of
a hassle, so instead I rewrote the contracts to be template-agnostic.
This change avoids a lot of the chaos associated with structural
differences between the fakes and non-fakes pipelines.
@kfindeisen kfindeisen requested a review from mrawls May 4, 2022 18:16
Copy link
Contributor

@mrawls mrawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments only, basically looks good, thank you!

pipelines/ApVerify.yaml Show resolved Hide resolved
pipelines/ApVerifyWithFakes.yaml Outdated Show resolved Hide resolved
pipelines/ApVerifyWithFakes.yaml Outdated Show resolved Hide resolved
After this change, most of the pipeline configuation is inherited
from ap_pipe.
These files should be the source of truth for all instrument-specific
configurations.
@kfindeisen kfindeisen merged commit db85c37 into main May 17, 2022
@kfindeisen kfindeisen deleted the tickets/DM-32694 branch May 17, 2022 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants