Opened 13 years ago

Closed 9 years 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 13 years ago.
ticket_15775.2.diff (4.7 KB ) - added by samufuentes 13 years ago.
ticket_15775.3.diff (4.8 KB ) - added by Bruno Gola 13 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 by Carl Meyer, 13 years ago

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 13 years ago by Carl Meyer (previous) (diff)

by samufuentes, 13 years ago

Attachment: ticket_15775.diff added

comment:2 by samufuentes, 13 years ago

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

comment:3 by Aymeric Augustin, 13 years ago

Needs tests: set

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

by samufuentes, 13 years ago

Attachment: ticket_15775.2.diff added

comment:4 by samufuentes, 13 years ago

Needs tests: unset

The second file includes tests

comment:5 by Aymeric Augustin, 13 years ago

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).

by Bruno Gola, 13 years ago

Attachment: ticket_15775.3.diff added

comment:6 by Bruno Gola, 13 years ago

Owner: changed from nobody to Bruno Gola
Status: newassigned

applied (some of) the changes proposed

comment:7 by anonymous, 13 years ago

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 by anonymous, 13 years ago

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

comment:9 by Claude Paroz <claude@…>, 9 years ago

In 58afd30b593fbd1e3d17c1bba83597da07bdb5f5:

Tested DecimalField with scientific notation

Refs #15775.

comment:10 by Claude Paroz, 9 years ago

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