remove sub-seconds from stats#147
Conversation
Signed-off-by: Zack Koppert <zkoppert@github.com>
Signed-off-by: Zack Koppert <zkoppert@github.com>
… for output Signed-off-by: Zack Koppert <zkoppert@github.com>
spier
left a comment
spier
left a comment
There was a problem hiding this comment.
Ran this locally, and it looks good.
Also looked through the code changes, and they LGTM.
Left one comment with a question.
| continue | ||
| if label not in time_in_labels: | ||
| time_in_labels[label] = [issue.label_metrics[label]] | ||
| time_in_labels[label] = [issue.label_metrics[label].total_seconds()] |
There was a problem hiding this comment.
The rest of this PR is add numpy.round() and some cosmetic changes.
Just curious, why did you have to add the .total_seconds here?
There was a problem hiding this comment.
This was definitely a mistake. In fact if the label is not in time_in_labels then I don't think we should be doing anything! I'll fix the total_seconds part here and remove the whole line in a separate PR with some testing.
There was a problem hiding this comment.
Well, I was wrong. 😄 This is needed. This part of the code stores the timedelta information for a given label and instead of storing it as a timedelta object, i'm choosing to store it as a number representing total_seconds so that round and averaging is easier. It is later converted back into a timedelta object after the math operations are complete.
2 participants
Proposed Changes
remove sub-seconds from stats. Addresses part of #144
Readiness Checklist
Author/Contributor
make lintand fix any issues that you have introducedmake testand ensure you have test coverage for the lines you are introducingReviewer
bug,documentation,enhancement,infrastructure, orbreaking