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-27869: Create jointcal pipetask #171
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
isullivan
approved these changes
Feb 22, 2021
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. My comments are mostly about documentation, though there is one place you should add a check for a value of None.
parejkoj
force-pushed
the
tickets/DM-27869
branch
from
March 2, 2021 20:54
f02adb0
to
d6d2b07
Compare
isullivan
approved these changes
Mar 2, 2021
isullivan
approved these changes
Mar 2, 2021
Add sample parquet visit-level file to test against, and tests of DataFrame->afw.SourceCatalog conversion using it. Rework gen2 loading to move source selection out of _build_ccdImage, because gen3 source selection happens on the visit-level DataFrame.
* Use visit-level catalog and exposure summary for gen3 input * Refactor gen2 write methods to share a single method we can use for gen3.
We can't use cmdline args like this in gen3, so if we want to keep it, it needs to be a config option. There are plans to allow a config to be split into sections by usage; I've left a comment to that effect for one block.
* Use pipeline definition file in jointcal tests. * Add pipeline definition file for HSC test. * Add gen3 test config. * Cleanup gen3 test config handling.
The cfht_minimal test was failing after the gen3 refactor because we previously created empty ccdImages, whereas now we just skip them if no sources were selected, resulting in a different, less obvious exception.
parejkoj
force-pushed
the
tickets/DM-27869
branch
from
March 3, 2021 00:57
b72a959
to
15505ad
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As implemented here, the single gen3 test doesn't test anything other than that the gen3 jointcal codepath runs without raising. I need to implement the new system of outputting measurements of Metrics, and figure out how to rework the existing code to work with a gen3 repo, since we cannot currently call pipetasks via an API, so I cannot get returned values from
JointcalTask.run()