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 localize keyword argument to DecimalField #4233

Merged
merged 4 commits into from
Jul 6, 2016

Conversation

kiyoqoko
Copy link
Contributor

Description

Added support for localized decimal and float fields.
Addresses #2897

@codecov-io
Copy link

codecov-io commented Jun 30, 2016

Current coverage is 91.20%

Merging #4233 into master will decrease coverage by 0.34%

@@             master      #4233   diff @@
==========================================
  Files            51         52     +1   
  Lines          5551       5779   +228   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           5082       5271   +189   
- Misses          469        508    +39   
  Partials          0          0          

Powered by Codecov. Last updated by 1d2fba9...cd40815

return float(value)
value = float(value)
if self.localize:
value = localize_input(value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Localizing the output doesn't make sense here. FloatField should represent as floats, not as a localized string.

@tomchristie
Copy link
Member

In a slight change to my previous comment the actual behavior that we'd want if we do decide to support this should be to only apply localization to numbers which are represented in string format.

Changes needed before we could consider this pull request

  1. Only apply to DecimalField.
  2. Add documentation.

@@ -923,7 +927,12 @@ def to_internal_value(self, data):
Validate that the input is a decimal number and return a Decimal
instance.
"""

if self.localize:
data = sanitize_separators(data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would make slightly more sense after the smart_text/.strip() line below.

@tomchristie tomchristie changed the title localize for DecimalField and FloatField Add localize keyword argument to DecimalField Jul 5, 2016
@tomchristie tomchristie added this to the 3.4.0 Release milestone Jul 6, 2016
@tomchristie
Copy link
Member

Nice work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants