-
Notifications
You must be signed in to change notification settings - Fork 533
MAINT: Refactor aggregate_outputs
for readability
#2969
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
MAINT: Refactor aggregate_outputs
for readability
#2969
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2969 +/- ##
==========================================
- Coverage 67.61% 64.23% -3.38%
==========================================
Files 344 342 -2
Lines 43798 43746 -52
Branches 5471 5468 -3
==========================================
- Hits 29612 28102 -1510
- Misses 13475 14549 +1074
- Partials 711 1095 +384
Continue to review full report at Codecov.
|
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.
Overall looks fine. I would strongly recommend against changing the exception types at this point, as it's just asking to break downstream code for very little benefit in clarity.
Two situations where you modify explicitly variables that would be modified implicitly by the functions that you pass them to. Feel free to reject those suggestions.
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
Just a rewrite for improved readability.
Acknowledgment