Skip to content
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

Merged
merged 3 commits into from
Jun 18, 2018

Conversation

Changaco
Copy link
Contributor

This branch adds a strict flag to the parse_decimal function. When set to True an exception is raised if the input number is formatted in a weird way. This is determined by comparing the string to the result of format_decimal().

Closes #589.

@codecov-io
Copy link

codecov-io commented Jun 16, 2018

Codecov Report

Merging #590 into master will decrease coverage by 0.01%.
The diff coverage is 88.23%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
babel/numbers.py 97.07% <88.23%> (-0.48%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3ef3c0...b5f6f36. Read the comment docs.

Copy link
Member

@akx akx left a 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
Copy link
Member

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, '')
Copy link
Member

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?

@Changaco Changaco force-pushed the strict-decimal-parsing branch from ff24ed9 to 71d80d6 Compare June 17, 2018 07:57
@Changaco Changaco force-pushed the strict-decimal-parsing branch from 71d80d6 to b5f6f36 Compare June 17, 2018 08:47
@Changaco
Copy link
Contributor Author

Ready for second review.

Copy link
Member

@akx akx left a 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!

@akx akx merged commit 23ca4bf into python-babel:master Jun 18, 2018
@Changaco Changaco deleted the strict-decimal-parsing branch June 18, 2018 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants