DecimalField allows float value as 'default'.

[Shawn Milochik]

Someone on django-users just commented that they set a default value on
a DecimalField as a float, and were surprised when they were unable to
create a queryset using a float to find records.

I searched Trac for the terms 'DecimalField default' and didn't see
anywhere that this was brought up before.

Should there be a validator added to the DecimalField that checks
whether decimal.Decimal(default) is a valid operation if default isn't
NOT_PROVIDED? Or maybe an override to __init__ in DecimalField that
calls super().__init__ then does this test.

Currently, it appears that this works because the value of 'default' is
coerced to unicode by Field's get_default(). This results in an invalid
value (float) being treated as valid for a DecimalField.

Shawn

[Russell Keith-Magee]

If DecimalField is accepting anything other than a DecimalField (or a
string, for historical reasons -- but that's true of any Field) under
*any* circumstances (default, new field value, etc), then it should be
raising an error. A float can't be reliably converted to a Decimal,
because you don't have guaranteed precision.

If this hasn't already been reported, then it should be. I have a
vague recollection of something about DecimalField defaults coming up
in the last year or so (either on Trac or Django-dev), but that could
just be a random pre-coffee neuron firing... I'll defer to anyone who
has actually done the Trac search :-)

Yours
Russ Magee %-)

[Shawn Milochik]

I've opened a ticket:
http://code.djangoproject.com/ticket/16015

I'd like to work on it. To have the best chance of my patch being
accepted, what's the best way to approach this? I believe the best thing
is to check that Decimal(value) works or raise a TypeError, and the
place would be to override __init__ in DecimalField, call the
super().__init__, and then do this check.

Thanks,
Shawn

[Russell Keith-Magee]

Well, __init__ won't help you, because that's the constructor for the
field instance, which is only used by the Meta class. When you've got
an instance of an object with a DecimalField on it, you don't have a
unique instance of DecimalField constructed with __init__.

I suspect the answer will lie somewhere between get_prep_value() and
to_python() on DecimalField, but you'll need to do some code path
tracing to confirm. Looking at the code, I'm a little surprised that
it doesn't work as is...

Yours,
Russ Magee %-)

[Shawn Milochik]

On 05/12/2011 08:04 PM, Russell Keith-Magee wrote:
>
> I suspect the answer will lie somewhere between get_prep_value() and
> to_python() on DecimalField, but you'll need to do some code path
> tracing to confirm. Looking at the code, I'm a little surprised that
> it doesn't work as is...
>
> Yours,
> Russ Magee %-)
>

Russell,

Thanks for the comments. I did dig into the code after my last e-mail
and I'm starting to understand how it works. It may be where you say,
and I see why it's definitely not a case of overriding the __init__
function.

The reason it is so liberal in accepting values at the moment is that
get_default function, which turns the default value into unicode before
returning it. I have a patch half-done (or so). Frustratingly, when I
try checking to see if f.has_default(), it returns True even if it's a
NOT_PROVIDED object. If I import NOT_PROVIDED into the validation and
check if "f.default is NOT PROVIDED" then it works. I'll report back
when I'm further along.

Shawn

[Shawn Milochik]

http://code.djangoproject.com/ticket/16015

I've uploaded a patch, but changed the status to 'Design Decision Needed' from 'Accepted.'

The problem is that although my patch follows the existing convention and results in passing tests in Python 2.6, the Python core developers have made an important change to the way Decimal objects work -- they may now be instantiated with floats. This means that my patch introduces a failing test if Python 2.7 is being used.

The patch:

http://code.djangoproject.com/attachment/ticket/16015/16015_DecimalField_invalid_defaults.diff

As I mentioned in the comment I just added to the ticket, the current "bug" is more pedantry than something that causes errors for real users. So, I leave it to the core devs to suggest an improved patch method or marking it wontfix.

Thanks,

Shawn