Opened 9 years ago
Closed 4 years ago
#25408 closed Cleanup/optimization (needsinfo)
Pass additional arguments to BaseForm.__init__ from BaseModelForm.__init__
Reported by: | Moritz Sichert | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | form modelform args |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Consider the following module "myapp.forms":
class CustomForm(BaseForm): def __init__(self, **kwargs): self.custom_kwarg = kwargs.pop('custom_kwarg', 'not available') def is_valid(self): if self.custom_kwarg: return True else: return super().is_valid() class CustomModelForm(ModelForm, CustomForm): pass
This module adds additional functionality to forms by adding a new keyword argument. If you want to use the new feature on a ModelForm too, that won't work because BaseModelForm.__init__()
doesn't capture additional arguments.
You can work around that like follows:
class CustomFormArg(BaseForm): def __init__(self, **kwargs): self.custom_kwarg = kwargs.pop('custom_kwarg', False) class CustomFormMethod(BaseForm): def is_valid(self): if self.custom_kwarg: return True else: return super().is_valid() class CustomForm(CustomFormArg, CustomFormMethod): pass class CustomModelForm(CustomFormArg, ModelForm, CustomFormMethod): pass
But I think it makes more sense to let BaseModelForm pass the arguments to the super() call. Maybe it would also be better if BaseModelForm only took **kwargs
instead of listing all arguments again.
Note: In some cases this can be fixed by reversing the bases of CustomModelForm:
class CustomModelForm(CustomForm, ModelForm): pass
However then the mro would be:
myapp.forms.CustomModelForm myapp.forms.CustomForm django.forms.models.ModelForm django.forms.models.BaseModelForm django.forms.forms.BaseForm object
instead of the more correct
myapp.forms.CustomModelForm django.forms.models.ModelForm django.forms.models.BaseModelForm myapp.forms.CustomForm django.forms.forms.BaseForm object
Change History (4)
comment:1 by , 9 years ago
comment:2 by , 9 years ago
The reason you need to inherit it from BaseForm
becomes more obvious when you look at the mro of the forms. Suppose you have a UserForm
. Normally its mro will look like this:
UserForm django.forms.forms.BaseForm object
Now you want to add functionality to all forms in your project so you create a new mixin called ProjectFormMixin
. If you then have a new MyProjectBaseForm
like this:
class MyProjectBaseForm(ProjectFormMixin, BaseForm): def has_changed(self): # custom detection if form has changed that also calls # super().has_changed() at some point
And UserForm
's mro (if you inherit it from MyProjectBaseForm
) will now be:
UserForm MyProjectBaseForm ProjectFormMixin django.forms.forms.BaseForm object
However if you also want to use this mixin in a model form base like this:
class MyProjectBaseModelForm(ProjectFormMixin, BaseModelForm): pass
the mro of a UserModelForm
will be:
UserModelForm MyProjectBaseModelForm ProjectFormMixin django.forms.models.BaseModelForm django.forms.forms.BaseForm object
when instead you wanted ProjectFormMixin
to be right above django.forms.forms.BaseForm
.
That's because super()
calls inside the ProjectFormMixin
will then hit BaseForm
as intended instead of BaseModelForm
.
Also super()
calls form within BaseModelForm
(they only relevant one is in BaseModelForm.__init__()
) will then hit your ProjectFormMixin
instead of the other way around.
comment:3 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I admit I haven't taken time to fully understand the issue, but I think if you can propose a patch, we'll take a look.
comment:4 by , 4 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
Closing as "needsinfo", because there is no way to move it forward without an extra explanation or a patch proposition. I'm also not sure if this is a valid issue because inheriting from both BaseForm
and BaseModelForm
simultaneously can be tricky and it's not clear why we should support it.
Do you need to inherit
CustomForm
fromBaseForm
? Could you inherit fromobject
and use it as a mixin on theModelForm
? Pardon my ignorance as I don't have much experience with multiple form inheritance. Do we have any documentation about it?