Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#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 ModelForms 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 of ModelForm.Meta.widgets (I don't like where this leads)
  • addition of an overrides attribute to ModelForm.Meta which can encompass the existing widgets 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)

17924.diff (13.2 KB ) - added by Simon Meers 12 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 by Simon Charette, 12 years ago

Cc: charette.s@… added

I agree that this could be useful but can't you just override __init__ and access self.fields directly?

class MyModel(models.Model):
  is_cool = models.BooleanField()
  content = models.CharField(max_length=255)

class MyForm(forms.ModelForm):
  class Meta:
    model = MyModel
  
  def __init__(self, *args, **kwargs):
    super(MyForm, self).__init__(*args, **kwargs)
    self.fields['is_cool'].widget.attrs.update(attr='value')
    self.fields['content'].widget = forms.TextArea()

comment:2 by Simon Meers, 12 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 Simon Charette, 12 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()

Or directly call formfield which is a bit ugly

self._meta.model._meta.get_field('fk_field').formfield(form_class=ModelChoiceFieldWithDescriptions)
Last edited 12 years ago by Simon Charette (previous) (diff)

comment:4 by Simon Meers, 12 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 Simon Charette, 12 years ago

Triage Stage: UnreviewedAccepted

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 Simon Charette, 12 years ago

Needs documentation: set
Needs tests: set

comment:7 by Simon Meers, 12 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 Jannis Leidel, 12 years ago

Resolution: wontfix
Status: newclosed

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 Simon Meers, 12 years ago

Attachment: 17924.diff added

comment:10 by Tai Lee, 12 years ago

Cc: real.human@… added

comment:11 by loic84, 11 years ago

Duplicate of #20000 to some extent.

Note: See TracTickets for help on using tickets.
Back to Top