Opened 6 years ago

Closed 22 months ago

#15775 closed Bug (fixed)

Can't enter scientific notation in decimal fields

Reported by: gregthe1 Owned by: Bruno Gola
Component: Forms Version: 1.2
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Here is an example:

>>> from django.forms import DecimalField
>>> f = DecimalField(max_digits=10, decimal_places=1)
>>> f.validate(Decimal('1E+2'))
Traceback (most recent call last):
...
ValidationError: [u'Ensure that there are no more than 1 decimal places.']

Also if I try upping the max_digits and decimal_places to something like 20, and 13, I get this error:

quantize result has too many digits for current context

Attachments (3)

ticket_15775.diff (2.9 KB) - added by samufuentes 6 years ago.
ticket_15775.2.diff (4.7 KB) - added by samufuentes 6 years ago.
ticket_15775.3.diff (4.8 KB) - added by Bruno Gola 5 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by Carl Meyer

Component: UncategorizedForms
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

The validation issue is a bug in DecimalField. DecimalField calls value.as_tuple() and unconditionally takes the absolute value of the third element (exponent) as the number of decimal places. This is just wrong, because it's perfectly valid for exponent to be positive, and in that case it doesn't represent a number of decimal places at all, it represents trailing zeros.

The other issue (quantize error) I can't reproduce and I don't think is related. Please open as a separate ticket with full reproduction instructions (including version of Python, since that's an error coming directly from Python's decimal module).

Last edited 6 years ago by Carl Meyer (previous) (diff)

Changed 6 years ago by samufuentes

Attachment: ticket_15775.diff added

comment:2 Changed 6 years ago by samufuentes

Easy pickings: unset
Has patch: set
UI/UX: unset

comment:3 Changed 6 years ago by Aymeric Augustin

Needs tests: set

It should be fairly easy to convert the example given in the description of the ticket into a test case.

Changed 6 years ago by samufuentes

Attachment: ticket_15775.2.diff added

comment:4 Changed 6 years ago by samufuentes

Needs tests: unset

The second file includes tests

comment:5 Changed 6 years ago by Aymeric Augustin

Patch needs improvement: set

I suggest the following changes to the patch, by order of decreasing importance.

You should use TestCase from django.utils.unittest, which is already imported, and not from django.test, since you're not using the extra features of django.test.TestCase.

            f.validate(Decimal('1E+2'))  # Ensure that scientific notation with positive exponent is accepted (#15775)

would be more readable as:

            # regression test for #15775: scientific notation with positive exponent is valid
            f.validate(Decimal('1E+2'))

You may try to re-wrap a bit django/forms/fields.py. It's hard to read 140-chars lines — the coding rules recommend wrapping at 80 chars.

You're missing a \n at the end of the file (look at your patch in Trac).

Changed 5 years ago by Bruno Gola

Attachment: ticket_15775.3.diff added

comment:6 Changed 5 years ago by Bruno Gola

Owner: changed from nobody to Bruno Gola
Status: newassigned

applied (some of) the changes proposed

comment:7 Changed 5 years ago by anonymous

I still don't think this is correct. Using the patch, it doesn't work for 1E+4.

>>> f = DecimalField(max_digits=10, decimal_places=1)
>>> f.validate(Decimal('1E+4'))
Traceback (most recent call last):
...
ValidationError: [u'Ensure that there are no more than 1 decimal places.']

comment:8 Changed 5 years ago by anonymous

Sorry, disregard my last comment. I was using the wrong version.

comment:9 Changed 22 months ago by Claude Paroz <claude@…>

In 58afd30b593fbd1e3d17c1bba83597da07bdb5f5:

Tested DecimalField with scientific notation

Refs #15775.

comment:10 Changed 22 months ago by Claude Paroz

Resolution: fixed
Status: assignedclosed

Seems the issue has been fixed since. I added a test to confirm that.

Note: See TracTickets for help on using tickets.
Back to Top