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

Fix errors introduced in previous rebase. #86

Merged
merged 1 commit into from May 12, 2021
Merged

Conversation

jeffcarlin
Copy link
Collaborator

Minor edits to fix changes that got missed during rebase/merge of DM-29841.

Copy link
Contributor

@SimonKrughoff SimonKrughoff 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.

@@ -2,7 +2,7 @@ description: Complete metrics measurement pipeline
imports:
- location: $FARO_DIR/pipelines/metrics_pipeline_matched.yaml
- location: $FARO_DIR/pipelines/metrics_pipeline_matched_multi.yaml
- location: $FARO_DIR/pipelines/metrics_pipeline_visit.yaml
# - location: $FARO_DIR/pipelines/metrics_pipeline_visit.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't leave commented out code, even in yaml files, I don't think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed - I think (and Dan T.'s questions in the thread on #dm-hsc-reprocessing are reminding me) that we need to settle on a standard set of pipelines, and add documentation (perhaps in the .yaml itself?) to explain what setting each pipeline is meant for. Otherwise we have a "main" pipeline in which we comment/uncomment things as needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will create a separate ticket to reorganize and improve documentation/readability of pipelines.

@@ -3,5 +3,7 @@ tasks:
matchCatalogsPatchMultiBand:
class: lsst.faro.preparation.PatchMatchedMultiBandPreparationTask
config:
connections.photoCalibName: fgcm_photoCalib
apply_external_wcs: True # We only support jointcal for now
python: |
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking over the pipeline used by validate_drp_gen3, I don't think this will break that, but just want to make sure.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, good point. I'll look up what validate_drp_gen3 uses.

@jeffcarlin jeffcarlin merged commit 1f9a259 into master May 12, 2021
@jeffcarlin jeffcarlin deleted the tickets/DM-30109 branch May 12, 2021 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants