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

Larger floats incorrectly pass DecimalField validation #3222

Closed
ryankask opened this issue Aug 5, 2015 · 4 comments
Closed

Larger floats incorrectly pass DecimalField validation #3222

ryankask opened this issue Aug 5, 2015 · 4 comments
Labels
Milestone

Comments

@ryankask
Copy link
Contributor

ryankask commented Aug 5, 2015

Larger floats are not failing DecimalField validation.

For example:

>>> serializers.DecimalField(max_digits=3, decimal_places=1).run_validation(200000000000.0)
>>> Decimal('2E+11')

I would expect a ValidationError to be raised since that number has more than 3 digits. Django's form validation throws the expected error:

>>> forms.DecimalField(max_digits=3, decimal_places=1).clean(200000000000.0)
ValidationError: [u'Ensure that there are no more than 3 digits in total.']

ValidationError is raised if you drop a zero from the above sample input:

>>> serializers.DecimalField(max_digits=3, decimal_places=1).run_validation(20000000000.0)
ValidationError: [u'Ensure that there are no more than 3 digits in total.']

Similarly, the exception is also raised using the original number but as a Decimal and int:

>>> serializers.DecimalField(max_digits=3, decimal_places=1).run_validation(200000000000)
ValidationError: [u'Ensure that there are no more than 3 digits in total.']
>>> serializers.DecimalField(max_digits=3, decimal_places=1).run_validation(Decimal('200000000000.0'))
ValidationError: [u'Ensure that there are no more than 3 digits in total.']

A key line recently changed in #2948 but it doesn't seem correct and deviates from Django's forms.DecimalField validation which otherwise appears to have been copied verbatim:

decimals = exponent * decimal.Decimal(-1) if exponent < 0 else 0

I don't really understand the original issue addressed in #2948 so I don't know why the line changed. I'm more than happy to work on the issue if I understand the original issue.

Below is a patch with a failing test:

diff --git a/tests/test_fields.py b/tests/test_fields.py
index 0427873..cf41a5b 100644
--- a/tests/test_fields.py
+++ b/tests/test_fields.py
@@ -773,6 +773,7 @@ class TestDecimalField(FieldValues):
         (Decimal('Nan'), ["A valid number is required."]),
         (Decimal('Inf'), ["A valid number is required."]),
         ('12.345', ["Ensure that there are no more than 3 digits in total."]),
+        (200000000000.0, ["Ensure that there are no more than 3 digits in total."]),
         ('0.01', ["Ensure that there are no more than 1 decimal places."]),
         (123, ["Ensure that there are no more than 2 digits before the decimal point."])
     )
@tomchristie tomchristie added the Bug label Aug 5, 2015
@tomchristie
Copy link
Member

Thanks Ryan.

@tomchristie
Copy link
Member

Now resolved, with some much more transparent & obvious logic.

@tomchristie tomchristie added this to the 3.2.0 Release milestone Aug 6, 2015
@Audace
Copy link

Audace commented May 24, 2020

@tomchristie @ryankask - I see this was added to a release milestone back in 2015. I'm still running into this issue on version 3.11.0. Did the fix end up getting released? Code below:

class RecommendationSerializer(serializers.Serializer):
    total_owed = serializers.DecimalField(decimal_places=2, max_digits=8, min_value='1.00',
                                          rounding=ROUND_DOWN)
    term_length = serializers.IntegerField(min_value=1)

    class Meta:
        fields = ['total_owed', 'term_length']


serializer = RecommendationSerializer(data={'total_owed': '12.333333333333333', 'term_length': 6})
serializer.is_valid(raise_exception=True)

>>> {"total_owed": ["Ensure that there are no more than 8 digits in total."]}

@xordoquy
Copy link
Collaborator

@Audace you get the expected validation error. The discussion group is the best place to take this discussion and other usage questions. Thanks!

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

No branches or pull requests

4 participants