Opened 7 years ago

Closed 4 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 Changed 7 years ago by Russell Keith-Magee

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 6 years ago by Matthias Kestenholz

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 Changed 6 years ago by Valentin Golev

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 Changed 6 years ago by Valentin Golev

Triage Stage: AcceptedDesign decision needed

comment:5 Changed 6 years ago by Łukasz Rekucki

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 Changed 5 years ago by Luke Plant

Severity: Normal

comment:7 Changed 5 years ago by Julien Phalip

Type: Bug

comment:8 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:9 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:10 Changed 4 years ago by Claude Paroz

Resolution: wontfix
Status: newclosed

Closing based on comment:5

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