[Django] #27317: Form.Media inheritance
Django
----------------------------+--------------------
Reporter: James Pic | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
I wanted to define a mixin to use with Form classes that would add extra
JS. So, I tried how Form.Media would behave with inheritance, and it turns
out they don't merge. For example:
{{{
class A(Form):
class Media:
js = ('jquery.js', 'a.js')
class B(Form):
class Media:
js = ('jquery.js', 'b.js')
class C(A, B):
pass
}}}
I expected:
{{{
C.Media.js == ('jquery.js', 'a.js', 'b.js')
}}}
But that results instead in:
{{{
C.Media.js = ('jquery.js', 'a.js')
}}}
Eventually, I wanted to push it a bit further and use a Mixin, having
exactly the same but with `A(object)` instead of `A(Form)`.
Can I work on a patch for this ?
Thanks for your support
--
Ticket URL: <https://code.djangoproject.com/ticket/27317>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-----------------------------+--------------------------------------
Reporter: James Pic | Owner: nobody
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by James Pic):
* needs_better_patch: => 0
* type: Bug => New feature
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27317#comment:1>
Django
-----------------------------+--------------------------------------
Reporter: James Pic | Owner: nobody
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I'm not sure if that's a good idea.
[https://docs.djangoproject.com/en/stable/topics/forms/modelforms/#form-
inheritance As the documentation says about Meta], "Normal Python name
resolution rules apply. If you have multiple base classes that declare a
Meta inner class, only the first one will be used. This means the child’s
Meta, if it exists, otherwise the Meta of the first parent, etc." There
was a [https://groups.google.com/d/topic/django-
developers/imjaHrfk6Eo/discussion slightly related thread] on django-
developers were it was discussed that model Meta inheritance should be
explicit rather than automatic. Can you think of any precedents in Django
where automatic merging similar to what you proposed takes place?
By the way, we're thinking about ways to replace `Media` with a better
solution, although no solution seems imminent (#22298).
--
Ticket URL: <https://code.djangoproject.com/ticket/27317#comment:2>
Django
-----------------------------+--------------------------------------
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Pic):
Thanks for re-centering the discussion. I understand this design decision.
However, explicit inheritance doesn't fix it:
{{{
In [3]: from django.forms.forms import Media
In [4]: class MediaA(Media):
...: js = ('jquery.js', 'a.js')
...:
In [5]:
In [5]: class MediaB(Media):
...: js = ('jquery.js', 'b.js')
...:
In [6]: class MediaC(MediaA, MediaB):
...: pass
...:
In [7]: MediaC.js
Out[7]: ('jquery.js', 'a.js')
}}}
Shouldn't we be expecting `MediaC.js = ('jquery.js', 'a.js', 'b.js')` here
? I can't think of any case where we wouldn't: this multiple inheritance
is after all, explicit.
--
Ticket URL: <https://code.djangoproject.com/ticket/27317#comment:3>
Django
-----------------------------+------------------------------------
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
I don't know, I guess we could see what the code looks like. I'm just wary
of investing time in something that's discouraged and/or headed toward
deprecation. Of course, I guess there are people love it.
--
Ticket URL: <https://code.djangoproject.com/ticket/27317#comment:4>
Django
-----------------------------+------------------------------------
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Pic):
Yes, because currently we can not know if we should or not add a media.
For example this would allow it:
{{{
class YourForm(forms.Form):
def add_media(self, media):
if not media.js.find('jquery.js'): # might have been added from
another app?
media.js.add('/yourapp/jquery.js')
media.js.add('b.js')
}}}
Also, probably we should be limited to one Media object per request.
--
Ticket URL: <https://code.djangoproject.com/ticket/27317#comment:5>
Django
-----------------------------+------------------------------------
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Pic):
Absolutely agree with Tim, which is why i didn't invest time on this, but
in adapters instead.
Full comment on the corresponding PR:
https://github.com/django/django/pull/9743
--
Ticket URL: <https://code.djangoproject.com/ticket/27317#comment:6>
Django
-----------------------------+------------------------------------
Component: Forms | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Johannes Hoppe):
Honestly, I would prefer the "normal" inheritance, where attributes
overwrite one another. I am not a big fan of the magic Django does in
`Model.Meta`. Its drives complexity and is inconsistent too. It makes
migrations and the state builder a pain... anyways. Forms. I do want to
repeat the same mistakes here.
Python overwrites attributes by default and I for one would want to keep
it that way.
--
Ticket URL: <https://code.djangoproject.com/ticket/27317#comment:7>