-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Cosmetic: improve exception traceback readability #80
Conversation
Codecov Report
@@ Coverage Diff @@
## main #80 +/- ##
==========================================
- Coverage 99.19% 99.06% -0.13%
==========================================
Files 9 9
Lines 741 745 +4
==========================================
+ Hits 735 738 +3
- Misses 6 7 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it 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.
Might as well fix some grammar while we're here. Otherwise, LGTM. I agree that this PR improves traceback readability.
@@ -124,7 +124,8 @@ def naturaldelta( | |||
""" | |||
tmp = Unit[minimum_unit.upper()] | |||
if tmp not in (Unit.SECONDS, Unit.MILLISECONDS, Unit.MICROSECONDS): | |||
raise ValueError(f"Minimum unit '{minimum_unit}' not supported") | |||
msg = f"Minimum unit '{minimum_unit}' not supported" |
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.
msg = f"Minimum unit '{minimum_unit}' not supported" | |
msg = f"Minimum unit '{minimum_unit}' not supported." |
raise ValueError( | ||
"Minimum unit is suppressed and no suitable replacement was found" | ||
) | ||
msg = "Minimum unit is suppressed and no suitable replacement was found" |
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.
msg = "Minimum unit is suppressed and no suitable replacement was found" | |
msg = "Minimum unit is suppressed, and no suitable replacement was found." |
I think the single sentence exception messages are fine without a full stop at the end. If there are two or more sentences, then it's needed. A quick search of CPython source code suggests the vast majority (~95%) do not end with a full stop: $ # Ending with a full stop
$ git grep "raise .*Error(.*\.['\"])" Lib | wc -l
79
$ # Ending with no full stop
$ git grep "raise .*Error(.*[^\.]['\"])" Lib | wc -l
1609 |
Changes proposed in this pull request: