-
Notifications
You must be signed in to change notification settings - Fork 12
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-41606: Option to output pipetask report info to command line #394
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #394 +/- ##
==========================================
+ Coverage 82.61% 82.62% +0.01%
==========================================
Files 91 91
Lines 10519 10529 +10
Branches 1993 1995 +2
==========================================
+ Hits 8690 8700 +10
Misses 1476 1476
Partials 353 353 ☔ View full report in Codecov by Sentry. |
eafc12d
to
c1f788d
Compare
@@ -218,11 +223,17 @@ def to_summary_dict(self, butler: Butler, do_store_logs: bool = True) -> dict[st | |||
- n_quanta_blocked: The number of quanta which failed due to | |||
upstream failures. | |||
- n_succeded: The number of quanta which succeeded. | |||
And possibly, if human-readable is passed: |
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 the doc error is from this line. You might need to add a blank line above and below.
67ad843
to
12578d6
Compare
b5c3bad
to
bb83d3d
Compare
|
||
Returns | ||
------- | ||
summary_dict : `dict` | ||
A dictionary containing a summary of a `TaskExecutionReport` for | ||
each task in the quantum graph. | ||
""" | ||
return { | ||
task: report.to_summary_dict(butler, do_store_logs=do_store_logs) |
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.
Why not just change this line to be
task: report.to_summary_dict(butler, do_store_logs=do_store_logs, human_readable=human_readable)
instead of adding the separate if/else statement?
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.
Thanks! I was thinking too hard about that one.
@@ -235,7 +248,17 @@ def to_summary_dict(self, butler: Butler, do_store_logs: bool = True) -> dict[st | |||
quantum_info["error"] = [ | |||
record.message for record in log if record.levelno >= logging.ERROR | |||
] | |||
failed_quanta[str(node_id)] = quantum_info | |||
if human_readable: | |||
failed_quanta["data_id"] = data_ids |
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.
This ought to be easy enough to fix; you could just repeat whatever test calls this function and explicitly set human_readable to True in one case and not in the other.
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.
Let's see if that works! I repeated the same tests (which should work, because the only difference between these reports is in the failed_quanta
section which we don't introspect into much). Update: it passes coverage! Wow, I was overthinking this.
e362e43
to
f8bbcaf
Compare
f8bbcaf
to
1173d2e
Compare
Changes in
pipe_base
: Add human-readable option to report summary dictionariesChecklist
doc/changes