#17924 closed New feature (wontfix)
Proposal: DRY/elegant/flexible ModelForm field attribute overrides
Reported by: | Simon Meers | Owned by: | Simon Meers |
---|---|---|---|
Component: | Forms | Version: | 1.3 |
Severity: | Normal | Keywords: | modelform, attributes, override, DRY |
Cc: | charette.s@…, real.human@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently overriding certain attributes of fields in ModelForm
s is somewhat limited/inflexible unless you resort to using the undocumented and inelegant formfield_callback
functionality.
For example, you can change a field's widget class quite elegantly by using the ModelForm.Meta.widgets
dictionary. However if you want to change a different aspect such as label
, required
, form_class
, etc you have to either:
- replace the field with a manually defined one and lose all attributes inherited from the corresponding
Models.Field
(as described in the caveat note) - define a
formfield_callback
function (undocumented / inelegant)
Two solutions spring to mind:
- addition of
labels
,required
,form_classes
analogues ofModelForm.Meta.widgets
(I don't like where this leads) - addition of an
overrides
attribute toModelForm.Meta
which can encompass the existingwidgets
functionality (potentially deprecatable) but also provides the ability to override other field attributes. For example
class MyForm(forms.ModelForm): class Meta: overrides = { 'description': { 'widget': forms.Textarea, 'label': 'Message', }, 'start_date': {'label': 'Date Range'}, 'end_date': {'label': 'to'}, 'type': { 'form_class': ModelChoiceFieldWithDescriptions, 'empty_label': None, }, }
This keeps inherited attributes DRY, but allows easy/elegant overriding of arbitrary field attributes. The patch for this is actually quite simple, which I'm very happy to polish and provide, I just thought I'd first seek out opinions regarding the approach before investing time in it.
Attachments (1)
Change History (12)
comment:1 by , 13 years ago
Cc: | added |
---|
comment:2 by , 13 years ago
Yes, though:
- that is more code, and less elegant
- the form field has already been created at that point, so you can't change it's class, etc (e.g. the
form_class = ModelChoiceFieldWithDescriptions
above)
comment:3 by , 13 years ago
I agree that it's easier to define it using your overrides but you can achieve form field replacement this way.
class MyForm(forms.ModelForm): class Meta: model = MyModel def __init__(self, *args, **kwargs): super(MyForm, self).__init__(*args, **kwargs) self.fields['fk_field'] = ModelChoiceFieldWithDescriptions()
comment:4 by , 13 years ago
True. Though the former loses all attribute inheritance from the models.Field
and the latter just reinforces my point. IMHO the simplicity and flexibility of the changes required would not be burdensome to the core codebase, and the result would make the form customization interface much nicer.
comment:5 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Marking as Accepted since you convinced me such a feature would be useful.
Another reason why this could be a great addition to the form API is that there's no documented way of doing this yet while it's quite a common use case.
comment:6 by , 13 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
comment:7 by , 13 years ago
In the interest of flexibilty/DRYness, it might be nice to also allow field classes or callables as overrides
dictionary keys to target multiple fields. For example:
overrides = { models.DateField: { 'widget': MyCustomDateWidget, }, lambda f: f.name.startswith('question_'): { 'required': True, }, }
So a string dictionary key is a shortcut for lambda f: f.name == key
and a class dictionary key is a shortcut for lambda f: isinstance(f, key)
. This is still a fairly simple patch.
Thoughts?
comment:8 by , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
I'm violently -1 on the whole topic "meta programming form fields after they've been instantiated", it's a mess. Yes it would be more DRY, but also much harder to know how the hell the form fields are composed as. Just override the form field, it's not a huge deal code wise and much easier to grok.
by , 12 years ago
Attachment: | 17924.diff added |
---|
comment:9 by , 12 years ago
To be discussed here: https://groups.google.com/d/topic/django-developers/VLTUxGExl00/discussion
comment:10 by , 12 years ago
Cc: | added |
---|
I agree that this could be useful but can't you just override
__init__
and accessself.fields
directly?