-
Notifications
You must be signed in to change notification settings - Fork 453
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
Add a strict mode to parse_decimal()
#590
Conversation
Codecov Report
@@ Coverage Diff @@
## master #590 +/- ##
==========================================
- Coverage 90.3% 90.29% -0.02%
==========================================
Files 24 24
Lines 4105 4120 +15
==========================================
+ Hits 3707 3720 +13
- Misses 398 400 +2
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.
Thank you! I added a couple comments that would make this even more useful.
babel/numbers.py
Outdated
proper = format_decimal(r, locale=locale, decimal_quantization=False) | ||
if string != proper and string.rstrip('0') != (proper + decimal_symbol): | ||
raise NumberFormatError( | ||
"%r is not a properly formatted decimal number" % string |
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.
It would be super nice to have the expected value in the error here.
Maybe even
class MisparsedNumberError(NumberFormatError):
def __init__(self, message, original, parsed, expected):
super(MisparsedNumberError, self).__init__(message)
self.original = original
self.parsed = parsed
self.expected = expected
# ...
raise MisparsedNumberError(
"%r is not properly formatted; parsed as %r, expected %r" % (string, r, proper),
original=string,
parsed=r,
expected=proper,
)
so users could have more visibility into what went wrong.
babel/numbers.py
Outdated
try: | ||
return decimal.Decimal(string.replace(get_group_symbol(locale), '') | ||
.replace(get_decimal_symbol(locale), '.')) | ||
r = decimal.Decimal(string.replace(group_symbol, '') |
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.
Could the name r
be something more descriptive?
ff24ed9
to
71d80d6
Compare
71d80d6
to
b5f6f36
Compare
Ready for second review. |
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 now. Thank you!
This branch adds a
strict
flag to theparse_decimal
function. When set toTrue
an exception is raised if the input number is formatted in a weird way. This is determined by comparing the string to the result offormat_decimal()
.Closes #589.