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-23302: Add ci_cpp package to allow calibration product generation to be tested #46

Merged
merged 11 commits into from Sep 11, 2020

Conversation

czwa
Copy link
Contributor

@czwa czwa commented Aug 12, 2020

Update cpCertify for upstream API changes.
Fix errors detected by ci_cpp code.

@czwa czwa requested a review from plazas August 13, 2020 21:44
Copy link
Contributor

@plazas plazas left a comment

Choose a reason for hiding this comment

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

I applaud the file name change from "blessCalibration.py" to "certifyCalibration.py". Could the same be done inside cpCertify? (or this would be too much trouble for this ticket?)

Comment on lines +70 to +77
certify = CertifyCalibration(butler=butler,
inputCollection=args.inputCollection,
outputCollection=args.outputCollection)

crozier.findInputs(args.datasetTypeName)
certify.findInputs(args.datasetTypeName)
if not args.skipCL:
crozier.addCalibrationLabel(beginDate=args.beginDate, endDate=args.endDate)
crozier.registerCalibrations(args.datasetTypeName)
certify.addCalibrationLabel(beginDate=args.beginDate, endDate=args.endDate)
certify.registerCalibrations(args.datasetTypeName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

certifier as a noun perhaps? Don't much care really as it's in a bin script though.

@@ -67,11 +67,11 @@
butler = Butler(args.root, run=args.inputCollection)
Copy link
Member

Choose a reason for hiding this comment

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

In the butler root above can you say "URI to butler" or something because it's not always a path.

@timj
Copy link
Member

timj commented Aug 20, 2020

As a general comment, we are trying to move away from lots of code in bin.src python scripts. They should follow the new approach where the bin.src just has two lines to call main in a script module. See how commands work in obs_lsst. This then lets you automatically include sphinx documentation for the command line tooling.

@czwa czwa force-pushed the tickets/DM-23302 branch 2 times, most recently from 781fe8e to bde7f37 Compare September 9, 2020 19:44
@czwa czwa merged commit beb4550 into master Sep 11, 2020
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

4 participants