Opened 13 years ago
Closed 13 years ago
#16015 closed Bug (wontfix)
DecimalField allows invalid default values.
Reported by: | ShawnMilo | Owned by: | ShawnMilo |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Design decision needed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
For example, a float may be passed in a DecimalField instantiation. This "works" because get_default coerces to unicode.
Attachments (1)
Change History (8)
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 13 years ago
comment:3 by , 13 years ago
Owner: | changed from | to
---|
by , 13 years ago
Attachment: | 16015_DecimalField_invalid_defaults.diff added |
---|
Decimal validation patch, DEMO ONLY -- do not apply, pending design decision
follow-up: 5 comment:4 by , 13 years ago
Triage Stage: | Accepted → Design decision needed |
---|
I've created a patch that works, but there's a problem. As of Python 2.7, you're suddenly allowed to instantiate a decimal.Decimal object with a float value.
http://docs.python.org/library/decimal.html
With Python2.6 my patch will result in a passing test. With Python 2.7, it will fail:
====================================================================== FAIL: runTest (__main__.InvalidModelTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "./runtests.py", line 93, in runTest self.assertTrue(not missing, "Missing Errors: " + '\n'.join(missing)) AssertionError: False is not True : Missing Errors: invalid_models.fielderrors: "decimalfield6": DecimalFields require a "default" attribute that is of type Decimal. --------------------------------"--------------------------------------
So, is it better to keep the currently technically-wrong-but-working state? I contacted the user who originally reported the confusion and they said they were using Python 2.6.1, so everything is "fine" unless they then try to create a queryset using a float.
comment:5 by , 13 years ago
Replying to ShawnMilo:
I've created a patch that works, but there's a problem. As of Python 2.7, you're suddenly allowed to instantiate a decimal.Decimal object with a float value.
It's hard to believe someone looked at the following example in the Python docs:
>>> Decimal(3.14) Decimal('3.140000000000000124344978758017532527446746826171875')
and thought "that's a really good idea". And yet...
So, is it better to keep the currently technically-wrong-but-working state? I contacted the user who originally reported the confusion and they said they were using Python 2.6.1, so everything is "fine" unless they then try to create a queryset using a float.
I'm torn on this one. On the one hand, it is desirable to be consistent across Django versions. It is also desirable to compensate for the inconsistencies across versions in Python as a language. However, in this case, the inconsistency appears to have been deliberately introduced, and will presumably be the "one true way" going forward, which is in conflict with what would seem to be to be the right behavior.
My personal preference would be to retain the purity of the "float != Decimal" distinction; but that would put Django at conflict the declared direction of Python as a language, and I don't think it's Django's place to change the expectations of the underlying language. My inclination is that the best approach here will be to catch the error under Python 2.6, and use the float_to_decimal implementation described in the Decimal FAQ.
comment:6 by , 13 years ago
Russell,
Since there's currently no error as-is in Python 2.6 or 2.7 (unless we introduce the patch above), where are you suggesting that we implement the float_to_decimal functionality?
Part of my confusion with this ticket is that get_default always returns a unicode object anyway -- for all field types. That could either be seen as a design flaw or as a "failsafe" because database backends all know how to coerce a string to the required type.
If get_default is as it should be, then it appears that this ticket should be closed with 'wontfix' unless I'm missing something.
comment:7 by , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
UI/UX: | unset |
I confirm that Python >= 2.7 allows instantiating a Decimal
from a float
:
% python2.6 >>> from decimal import Decimal >>> Decimal(0.5) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/opt/local/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/decimal.py", line 649, in __new__ "First convert the float to a string") TypeError: Cannot convert float to Decimal. First convert the float to a string % python2.7 >>> from decimal import Decimal >>> Decimal(0.5) Decimal('0.5') % python3.2 >>> from decimal import Decimal >>> Decimal(0.5) Decimal('0.5')
I'm a bit surprised by this change, but I understand the reasons, and I think it's better to follow Python's lead here.
Also, a developer who uses DecimalField
instead of FloatField
is probably conscious of the difference between a Decimal
and a float
, and will know why DecimalField(default=0.3)
isn't a good idea.
Accepting ticket based on Russell's comment on the mailing list:
http://groups.google.com/group/django-developers/browse_thread/thread/637426c493f490d8