[Django] #16306: Validators not set for form field when max_value set in __init__
Django
-----------------------+---------------------------------
Reporter: ShawnMilo | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: Forms
Version: 1.3 | Severity: Release blocker
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------+---------------------------------
I've tested this for forms.IntegerField and forms.DecimalField -- setting
a max_value when the field is declared works normally, but if set in
__init__ (because it's unknown till runtime) no validators are assigned
although the max_value value is properly set.
{{{
from django import forms
class TestForm(forms.Form):
amount = forms.IntegerField(max_value = 10)
class TestForm2(forms.Form):
amount = forms.IntegerField()
def __init__(self, *args, **kwargs):
super(TestForm2, self).__init__(*args, **kwargs)
self.fields['amount'].max_value = 10
x = TestForm({'amount': 12})
print x.is_valid() #returns False
print x.fields['amount'].max_value # returns 10
print x.fields['amount'].validators #
[<django.core.validators.MaxValueValidator object at 0x7febd5240610>]
print x.errors #complains about size
x = TestForm2({'amount': 12})
print x.is_valid() #returns True
print x.fields['amount'].max_value #returns 10
print x.fields['amount'].validators # []
print x.errors #nothing
}}}
Evidently this bug has existed for some time, because almost a year ago it
was discussed on StackOverflow and the suggestion was to just completely
replace the field at runtime:
http://stackoverflow.com/questions/3470741/django-forms-integerfield-set-
max-value-on-runtime
--
Ticket URL: <https://code.djangoproject.com/ticket/16306>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: ShawnMilo | Owner: nobody
Type: Bug | Status: closed
Version: 1.3 | Severity: Release blocker
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+---------------------------------
Changes (by melinath):
* status: new => closed
* needs_docs: => 0
* resolution: => invalid
* needs_tests: => 0
* needs_better_patch: => 0
Comment:
max_value and min_value are documented as arguments to the !__init!__
method of the !DecimalField and !IntegerField. They are not documented as
attributes on the field instances. The don't even seem to be used
internally, so I'm not really sure why they even get set. In any case,
this is not a bug, and there are ways to change the field at runtime
already in place.
If you want to request this as a new feature, reopen the ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/16306#comment:1>
Django
attributes.
-------------------------------------+-------------------------------------
Reporter: ShawnMilo | Owner: nobody
Type: Bug | Status: reopened
Milestone: | Component: Documentation
Version: SVN | Severity: Normal
Resolution: | Keywords: form field
Triage Stage: Accepted | documentation
Needs documentation: 1 | Has patch: 0
Patch needs improvement: 0 | Needs tests: 1
UI/UX: 0 | Easy pickings: 1
-------------------------------------+-------------------------------------
Changes (by melinath):
* status: closed => reopened
* severity: Release blocker => Normal
* component: Forms => Documentation
* needs_tests: 0 => 1
* version: 1.3 => SVN
* easy: 0 => 1
* keywords: => form field documentation
* needs_docs: 0 => 1
* resolution: invalid =>
* stage: Unreviewed => Accepted
Comment:
Meh. Reread the
[https://docs.djangoproject.com/en/dev/ref/forms/fields/#django.forms.DecimalField.max_value
documentation] and although the words say "optional arguments" the
examples are indeed attributes. I'm marking this as a documentation bug. I
think that a feature request would be valid, but that's a separate issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/16306#comment:2>
Django
attributes.
-------------------------------------+-------------------------------------
Type: Bug | Status: reopened
Milestone: | Component: Forms
Resolution: | Keywords: form field
decision needed | Has patch: 0
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by ShawnMilo):
* component: Documentation => Forms
* stage: Accepted => Design decision needed
Comment:
Also, a comment about using one of the following workarounds:
1. Replace the field completely in the form's __init__.
2. Add the field in the form's __init__ (don't have it in the original
definition).
Both of these would cause confusion for the maintenance developer: Either
a field is appearing 'by magic' or it's not as defined in the form
originally. Neither case is desirable.
--
Ticket URL: <https://code.djangoproject.com/ticket/16306#comment:3>
Django
attributes.
-------------------------------------+-------------------------------------
Milestone: | Component: Forms
Version: SVN | Severity: Normal
Resolution: | Keywords: form field
Triage Stage: Design | documentation
decision needed | Has patch: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by SmileyChris):
* needs_tests: 1 => 0
* easy: 1 => 0
Comment:
How does this look?
--
Ticket URL: <https://code.djangoproject.com/ticket/16306#comment:4>
Django
attributes.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: form field | Triage Stage: Accepted
documentation | Needs documentation: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by jacob):
* stage: Design decision needed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/16306#comment:6>
Django
attributes.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: form field | Triage Stage: Accepted
documentation | Needs documentation: 1
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: bmispelon@… (added)
Comment:
I took a closer look at this ticket and investigated several approaches
(including the proposed patch)
I came up with 3 approaches on this ticket (the 3rd one being
SmileyChris's one), but all of them have some issue:
1) move the creation of validators from `__init__()` to `run_validators()`
This approach is the less intrusive one.
It works quite well but breaks some tests in the test suite, which
shows there are some potential backwards-compatibility issues.
The root of these issues is code which expects the field instance's
`validators` attribute to be populated before the field has been actually
validated [1].
There's also two possibilities with this approach, the difference
being whether we append the validators inside `run_validators` (in which
case a `MinValueValidator` would be placed after any custom one for
example), or whether we prepend them (this is more compatible with the
existing behavior).
2) Use lazy values
This sort of works, but there are two problems:
* The validators need to be changed because they can now be passed
a None value to validate against.
* The error messages are broken because `"%d" % lazy_int` does not
work (and as far as I know, that's not something that can be made to
work).
3) descriptors (proposed patch)
This approach is quite elegant on paper, and while the patch looks
good, I think the proposed solution can't work because of some new
features that were added since the patch was proposed.
It's a bit tricky to explain why, but let me try.
Let's consider what happens when creating an `IntegerField` with a
`max_value` argument:
* `IntegerField(max_value=42)` calls `IntegerField.__init__`
* The `__init__` needs to do two things: set `self.min_value` to
42, and call `Field.__init__`.
* Setting `self.min_value = 42` calls `ValidatorAttribute.__set__`
* `ValidatorAttribute.__set__` requires the field's `validators`
attributes (a list of validators) to exist.
* This attribute is set in `Field.__init__`
* This means that `IntegerField.__init__` needs to call
`Field.__init__` before setting `self.min_value`
* However ...
* `Field.__init__` calls `self.widget_attrs()`, which allows the
field to pass custom attributes to the widget.
* `IntegerField.widget_attrs` checks the value of `self.min_value`
...
* ... which hasn't been set for the reasons stated above.
It might be possible to add some more descriptors to make the whole
thing work.
For example, I tried making `Field.widget` into a property that would
only call `widget_attrs` when being accessed.
This however failed with a weird error message and it started getting
hard to trace the problem with all the different layers of descriptors.
All things considered, I'm not sure that adding all these additional
layers is worth it.
I think approach 1 is the best one, even at the cost of minor backwards-
compatibility.
`Field.validators` is not really documented and the documentation has no
examples
of directly manipulating it.
Additionally, other fields have attributes which could use a similar fix:
* CharField.max_length
* charField.min_length
* DecimalField.max_digits
* DecimalField.decimal_places
* RegexField.regex (current solution is based on @property)
[1]
https://github.com/django/django/blob/master/tests/forms_tests/tests/forms.py#L822
--
Ticket URL: <https://code.djangoproject.com/ticket/16306#comment:7>
Django
attributes.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Keywords: form field | Triage Stage: Accepted
documentation |
Comment (by Abhishek Mane):
Hi, I am looking to make my first contribution for gsoc 2026. I see this
has been marked as a documentation issue regarding how max_value and
min_value are described. I would like to work on updating the docs to
clarify that these are initialization arguments, not field attributes. Let
me know if I can assign this to myself and get started on a patch
--
Ticket URL: <https://code.djangoproject.com/ticket/16306#comment:8>
Django
attributes.
-------------------------------------+-------------------------------------
| Mane
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: form field | Triage Stage: Accepted
documentation |
Needs tests: 0 | Patch needs improvement: 0
Changes (by Abhishek Mane):
* has_patch: 0 => 1
* needs_docs: 1 => 0
* owner: nobody => Abhishek Mane
* status: new => assigned
Comment:
Submitted a PR to update the built-in fields documentation, clarifying
that validation arguments should be passed during initialization:
https://github.com/django/django/pull/20855
--
Ticket URL: <https://code.djangoproject.com/ticket/16306#comment:9>