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-38234: Improve DuplicateOutputError log message #313
Conversation
2996ba9
to
071a31e
Compare
071a31e
to
b3d73c3
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #313 +/- ##
==========================================
- Coverage 80.68% 80.65% -0.03%
==========================================
Files 57 57
Lines 6377 6379 +2
Branches 1303 1303
==========================================
Hits 5145 5145
- Misses 984 985 +1
- Partials 248 249 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
b3d73c3
to
c15fc69
Compare
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, just some clarity comments.
python/lsst/pipe/base/pipeTools.py
Outdated
"DatasetType `{}' in task `{}' already appears as an output in task `{}'.".format( | ||
dsTypeDescr.name, | ||
taskDef.label, | ||
dsTypeTaskLabels[dsTypeDescr.name], |
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 an f-string might be more readable here; it's a bit disorienting that the string is laid out horizontally while the substitution arguments are laid out vertically.
) | ||
dsTypeTaskLabels[dsTypeDescr.name] = taskDef.label |
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.
The fact that the test is against allOutputs
while the insertion is against dsTypeTaskLabels
is a bit awkward, logically. Is it already guaranteed that a task can't have two connections with the same dataset name?
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.
A single task cannot output the same dataset type twice. The important insertion is dsTypeDescr.name
into outputs
, which then gets subsumed into allOutputs
to facilitate future checks. The insertion you've highlighted here is merely a book-keeping exercise that keeps a record of the prior tasks which output a given dataset type.
c15fc69
to
f65404b
Compare
Checklist
doc/changes