Opened 14 years ago

Closed 12 years ago

#13075 closed Bug (wontfix)

BaseForm __init__ doesn't super and so breaks MI Mixins

Reported by: gbauer Owned by: nobody
Component: Forms Version: 1.1
Severity: Normal Keywords: mixins
Cc: me@…, Łukasz Rekucki Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

BaseForm's init method doesn't do a super(...) call to follow up the chain, which breaks writing mixins for ModelForms, as the chain is not followed to the classes to the right of the base classes:

class MyForm(ModelForm, ModelFormMixin):

... do stuff

Inheritance will follow ModelForm up the chain until it hits BaseForm and then stop following up further, but ModelFormMixin is in the mro after the mro of ModelForm and so will never looked at. Just adding a super(BaseForm, self).init(.....) to the BaseForm init will work wonders there ...

Change History (10)

comment:1 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Matthias Kestenholz, 14 years ago

This is a bit harder than it looks at first sight. According to http://fuhm.net/super-harmful/ , every single argument should be passed on. However, this obviously won't work when there aren't any mixins around, just object, because object does not accept any arguments in init.

On the other hand, standard subclassing of Form expects all arguments BaseForm itself accepts too.

Depending on the mro, mixins/subclasses will have to accept either *args/kwargs or nothing.

I'm not sure how we should fix this issue.

comment:3 by Valentin Golev, 13 years ago

Cc: me@… added

We could choose a class-based generic views way: add a self.args/self.kwargs parameter (or self._args/self._kwargs maybe)

comment:4 by Valentin Golev, 13 years ago

Triage Stage: AcceptedDesign decision needed

comment:5 by Łukasz Rekucki, 13 years ago

Cc: Łukasz Rekucki added

I would vote for "won't fix" on this. The proper way to write this is:

class MyForm(ModelFormMixin, ModelForm):
    pass

I don't see a reason why a mixin should be later in MRO then the class it extends. If the mixin is unrelated to ModelForm then putting it in front won't hurt either. This is also how class-based views use mixins.

comment:6 by Luke Plant, 13 years ago

Severity: Normal

comment:7 by Julien Phalip, 13 years ago

Type: Bug

comment:8 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:9 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:10 by Claude Paroz, 12 years ago

Resolution: wontfix
Status: newclosed

Closing based on comment:5

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