-
Notifications
You must be signed in to change notification settings - Fork 18
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-41442: Add focal plane mosaic construction to cp_pipe/cp_verify #935
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.
I think I see what's going on, but would greatly appreciate (a) more fully fleshed out comments; (b) more appropriate default connection names.
) | ||
|
||
outputExp = cT.Output( | ||
name="calexpBin", |
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.
Should the default name be biasBin
to go with bias
?
|
||
class VisualizeMosaicCalibConnections(pipeBase.PipelineTaskConnections, dimensions=("instrument",)): | ||
inputExps = cT.Input( | ||
name="calexpBin", |
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.
And I think this should be biasBin
to match.
) | ||
|
||
outputExp = cT.Output( | ||
name="calexpBin", |
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.
Name here.
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.
In fact the default should probably be flat
to signify that this is something over the flat (per filter) (all related connections).
Add variety of subclasses of task/config to support different input dimensions.