On 06/10/2011 02:45, Paul McMillan wrote:
.. (A) silent rounding issue:
when a decimal value is saved in db
its decimal digits exceeding decimal_places are rounded off using
.quantize(). Rounding defaults to ROUND_HALF_EVEN (python default).
There is no mention of this behavior in docs.
Docs patches welcomed. This rounding behavior is generally superior to
ROUND_UP (what most of us are taught in gradeschool) for a number of
reasons that are inappropriate to get into here. If we're going to
round, that's the behavior we should use as default.
Maybe you were referring to ROUND_HALF_UP, where 0.5 becomes 1?
with current ROUND_HALF_EVEN 0.5 becomes 0.
Yes I agree ROUND_HALF_EVEN in this particular situation might be
better than ROUND_HALF_UP, let's write it in docs.
.. (B) no model validation for decimal digits:
.full_clean() does not raise any exception:
- if the value has too many decimal digits
- or if value has too many digits
other fields, like CharField, do not validate values that exceed fields
constraints
There's no clearly defined way to deal with overlong CharFields.
Rounding overlong floating point numbers is common and not unexpected.
Decimals work similarly to floats, and so it's not unreasonable to
have similar behavior.
Reading the docs on Python's decimal module, it makes sense to think
of the decimal_places as an analog to python's decimal notion of
precision.
http://docs.python.org/library/decimal.html#module-decimal
.. (C) errors on save:
decimal values exceeding field's digits boundaries (decimal or total) make
.save() raise decimal.InvalidOperation exceptions
This does sound like an error in validation. If it's possible to pass
a value through form validation to the point where it has errors on
save, that is a bug.
Forms (ModelForms) correctly check decimal places and max_digits.
I was referring to model validation: .full_clean() on model instance
that in turn calls .validate() on field instance
In my opinion they should be fixed in a backwards-incompatible way!:
Thank you for your opinion, we don't do backwards-incompatible fixes.
There's usually a compatible way to fix behavior.
(A) django shuld not round decimal values silently before saving, if the
value has too many decimal digits,
raise an exception (just like for values that have too many digits)
In the general use case, the existing behavior is more user friendly.
Many people would run out and re-add the rounding behavior if we
changed it. Users will enter over-long strings into decimal fields.
It's really unfriendly to paste an 32 place decimal number in and be
told it can only be 17 decimal digits long (and then have to go count
out till you get the correct number).
This is exactly what happens right now with forms.DecimalField,
It requires Users to check length of decimal input: it's very picky
and it does a good job: 0.91 will not validate on a form field
with only one decimal digit.
I don't understand who are thu Users you mention...
Users of model.full_clean()
are usually programmers that get frustrated to find out that the model
did validate and then they get errors on save()
Users of web forms are not allowed to enter values with more decimal digits
than expected, even if the value will be rounded. As a matter of fact
forms.DecimalField does not know ronding.
That said, a no_rounding kwarg sounds like a perfectly reasonable feature.
ok, the kwarg will do
(B) decimalfield shoud not validate values that exceed its max_digits and
decimal_places constraints
I agree that decimalfield should not validate values that exceed
max_digits. If it does, that is a bug we should fix. We need to be
careful not to create situations where very large numbers validate but
very small but precise decimal numbers get rounded unexpectedly.
Unfortunately, these two parameters overlap in difficult ways. I
disagree about decimal_places, since the expected behavior is rounding
in most other real-world circumstances.
ok, that's fine, again let's wirite it in the docs: when you call save() your
decimal is rounded
with ROUND_HALF_EVEN to match decimal_places.
Does the combination of documenting the existing rounding behavior,
fixing the error on save() with max_digits, and adding the no_rounding
feature address your concerns?
yes, let's summarize:
- (A) add a no_rounding kwarg to models.DecimalField
- (B) implement models.DecimalField .validate() that will raise an exception
if value exceed decimal_places and no_rounding
OR if value exceeds max_digits.
The principle is: if models.DecimalField.validate() passes
then it's safe to call save()
in practice we could use code in forms.fields.DecimalField
as a starting point
- (C) errors on .save() are acceptable ONLY if .validate() raises exception
we keep rounding HALF_EVEN if value has too many decimal digits
we keep raising a decimal.InvalidOperation exception if value exceeds
max_digits
If we agree, I will be happy to write a patch with tests and docs
Thanks for your comments,
Marco
--
You received this message because you are subscribed to the Google Groups "Django
developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
