[Django] #25408: Pass additional arguments to BaseForm.__init__ from BaseModelForm.__init__
Django
--------------------------------------+---------------------------------
Reporter: MoritzS | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Severity: Normal | Keywords: form modelform args
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+---------------------------------
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25408>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: MoritzS | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: form modelform args | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Do you need to inherit `CustomForm` from `BaseForm`? Could you inherit
from `object` and use it as a mixin on the `ModelForm`? Pardon my
ignorance as I don't have much experience with multiple form inheritance.
Do we have any documentation about it?
--
Ticket URL: <https://code.djangoproject.com/ticket/25408#comment:1>
Django
Reporter: MoritzS | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: form modelform args | Triage Stage:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by MoritzS):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25408#comment:2>
Django
Reporter: MoritzS | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Keywords: form modelform args | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25408#comment:3>
Django
Reporter: Moritz Sichert | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: form modelform args | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => needsinfo
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25408#comment:4>