Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fixed #19617 -- Refactored form metaclasses to support more inheritance scenarios. #1382

Merged
merged 2 commits into from

5 participants

@loic
Collaborator

No description provided.

@jezdez
Owner

As mentioned on IRC, please don't rename the meta class just because you can, this is a critical part used by many apps out there, even if not public API. That'll keep upgraders happy

@loic
Collaborator

I might revisit this patch at a later stage.

@loic loic closed this
@loic
Collaborator

There have been discussions about allowing non-Field values to shadow Field instances as a way to "opt-out" from a declarative field: https://code.djangoproject.com/ticket/8620#comment:18

I added a new commit that addresses that. It however removes support for Fields declared on plain Python mixins from the previous commit. Although I think it's a good thing, as supporting them is ugly in term of implementation. Also the fields attributes declared on simple mixins would leak onto the child class, which is not desirable.

For Fields to be pickup up, the parent class needs to bear the DeclarativeFieldsMetaclass metaclass; forms.Form should work well as a "field holder" or we could introduce FormMixin as suggested in the ticket description.

Since Form is just an empty wrapper around BaseForm and ModelForm also inherits from BaseForm there are no consequences in using a Form as a field holder.

@loic loic reopened this
@mjtamlyn mjtamlyn commented on the diff
tests/model_forms/tests.py
@@ -1800,3 +1800,29 @@ def test_multiple_widgets(self):
html = form.as_p()
self.assertInHTML('<ul id="id_status">', html)
self.assertInHTML(dreaded_help_text, html, count=0)
+
+
+class ModelFormInheritanceTests(TestCase):
+ def test_form_subclass_inheritance(self):
+ class Form(forms.Form):
+ age = forms.IntegerField()
+
+ class ModelForm(forms.ModelForm, Form):
+ class Meta:
+ model = Writer
+ fields = '__all__'
+
+ self.assertEqual(list(ModelForm().fields.keys()), ['name', 'age'])
@mjtamlyn Collaborator

You can probably use assertSequenceEqual here which might be a bit nicer.

@loic Collaborator
loic added a note

I tried, it doesn't work on PY3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mjtamlyn
Collaborator

Conceptually this looks good to me. I'm happy with the restriction that field holders have to be Forms (or similar). I'd rather avoid introducing a new object - it makes sense to me that you can take one form and combine it with another to get a new form.

It might be nice to see a few more complex hierarchies in the tests to ensure that everything is working. Then we should just need some documentation work.

django/forms/forms.py
((37 lines not shown))
- return SortedDict(fields)
+ # Current class attributes.
+ current_fields = []
+ for key, value in attrs.items():
+ if isinstance(value, Field):
+ current_fields.append((key, value))
+ # Check for Field shadowing.
+ elif key in declared_fields:
@loic Collaborator
loic added a note

I would limit the shadowing to None (e.g. username = None) to denote a more explicit decision and I would document it along those lines: If you want to opt-out from a field declared by your base classes, set it to "None" on the current class.

@mjtamlyn Collaborator

I disagree - if you override a field name with anything (method, string, None, some other field) then it should work. This should behave like a normal python class - override an attribute and the class knows nothing about the parent's attribute at that name.

@loic Collaborator
loic added a note

Consider that we "remove" that attr, so it's not exactly a normal python pattern. If we accept any value, I wouldn't recommend doing the the delattr any more.

Edit: Also keep in mind that if we don't delattr the attribute, it will linger around as a value in further subclasses where it will hardly make any sense. IMO we should treat None the same way we treat a Field, it's a magical value, anything else shouldn't be attempted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
django/forms/forms.py
((46 lines not shown))
+ attrs.pop(key)
+ declared_fields.pop(key)
+ current_fields.sort(key=lambda x: x[1].creation_counter)
+
+ declared_fields.update(current_fields)
+
+ attrs.update({
+ 'base_fields': declared_fields,
+ 'declared_fields': declared_fields,
+ })
+
+ return (super(DeclarativeFieldsMetaclass, mcs)
+ .__new__(mcs, name, bases, attrs))
+
+
+class FormMetaclass(DeclarativeFieldsMetaclass):
@loic Collaborator
loic added a note

@jezdez: Is this an acceptable compromise in term of backward compatibility? Conceptually DeclarativeFieldsMetaclass is quite tricky and I'd like to keep it focused, having the media attribute (which is not a Declarative Field per se) handling in the middle doesn't help to make it any clearer. You make the call, I'll update accordingly.

@apollo13 Owner
apollo13 added a note

As discussed on IRC: no.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@apollo13
Owner

To summarize what's still needed:

  • Docs in the release notes
  • Leave get_declared_fieldsets as is and deprecate it (Django won't use it anymore, but let's give others a time to update their code)
  • Restore the unneeded renaming of the other classes.
@loic
Collaborator

This last commit deals with fields shadowing the same way Python would for standard attribute shadowing.

I consider this patch RFC.

django/forms/forms.py
((11 lines not shown))
"""
- def __new__(cls, name, bases, attrs):
- attrs['base_fields'] = get_declared_fields(bases, attrs)
- new_class = super(DeclarativeFieldsMetaclass,
- cls).__new__(cls, name, bases, attrs)
- if 'media' not in attrs:
- new_class.media = media_property(new_class)
+ def __new__(mcs, name, bases, attrs):
+ # Collect fields from current class.
+ current_fields = []
+ for key, value in attrs.items():
+ if isinstance(value, Field):
+ current_fields.append((key, value))
+ attrs.pop(key)
@funkybob
funkybob added a note

Won't this cause issues in Py3, as you're modifying a dict you're iterating? I had to wrap attrs.items() in list()
Also, why not just use:
current_fields.append((key, attrs.pop(key)))
?

@loic Collaborator
loic added a note

Good catch for PY3. I like the standalone attrs.pop(key) because it makes it standout. When dealing with metaclasses I really like making each line atomic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@loic
Collaborator

Rebased to resolve conflicts.

tests/model_forms/tests.py
((16 lines not shown))
+ self.assertEqual(list(ModelForm().fields.keys()), ['name', 'age'])
+
+ def test_field_shadowing(self):
+ class Mixin(object):
+ age = None
+
+ class Form(forms.Form):
+ age = forms.IntegerField()
+
+ class ModelForm(forms.ModelForm):
+ class Meta:
+ model = Writer
+ fields = '__all__'
+
+ self.assertEqual(list(ModelForm().fields.keys()), ['name'])
+ self.assertEqual(list(type(str('NewForm'), (Mixin, Form), {})().fields.keys()), [])
@apollo13 Owner

What about a test like:

self.assertEqual(list(type(str('NewForm'), (Form2, Mixin, Form), {})().fields.keys()), [])

Does Mixin still nuke out the age attr then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@loic
Collaborator

Rebased to latest master.

@apollo13
Owner

@mjtamlyn & @jezdez : Mind giving this a second look so we have another opinion?

@loic
Collaborator

Rebased again, another conflict.

@loic
Collaborator

Rebased / retested.

docs/topics/forms/modelforms.txt
@@ -534,8 +531,8 @@ field, you could do the following::
fields = ['pub_date', 'headline', 'content', 'reporter']
-If you want to override a field's default validators, then specify the
-``validators`` parameter when declaring the form field::
+If you want to specify a field's validators, you can do so by defining
+the field declaritively and setting its ``validators`` parameter::
@mjtamlyn Collaborator

declaratively

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
docs/topics/forms/modelforms.txt
((14 lines not shown))
-Chances are these notes won't affect you unless you're trying to do something
-tricky with subclassing.
+* It's possible to opt-out from a ``Field`` inherited from a parent class by
+ shadowing it. While any non-``Field`` value works for this purpose, it's
+ recommended to use ``None`` to make it explicit that a field is being
+ nullified.
@mjtamlyn Collaborator

Can we add a note here to say that you can't use this to opt out of a field which is only present because of the Metaclass?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mjtamlyn mjtamlyn commented on the diff
docs/topics/forms/modelforms.txt
((7 lines not shown))
-Chances are these notes won't affect you unless you're trying to do something
-tricky with subclassing.
+* It's possible to inherit from both ``Form`` and ``ModelForm`` simultaneuosly,
+ however, you must ensure that ``ModelForm`` appears first in the MRO. This is
+ because these classes rely on different metaclasses and a class can only have
+ one metaclass.
+
+.. versionadded:: 1.7
+
+* It's possible to opt-out from a ``Field`` inherited from a parent class by
+ shadowing it. While any non-``Field`` value works for this purpose, it's
+ recommended to use ``None`` to make it explicit that a field is being
+ nullified.
@mjtamlyn Collaborator

If you add the comment here as discussed, I'm happy to merge this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mjtamlyn mjtamlyn merged commit ce823d3 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 14, 2013
  1. @loic

    Fixed #19617 -- Refactored Form metaclasses to support more inheritan…

    loic authored
    …ce scenarios.
    
    Thanks apollo13, funkybob and mjtamlyn for the reviews.
  2. @loic
This page is out of date. Refresh to see the latest.
View
53 django/forms/forms.py
@@ -11,7 +11,7 @@
from django.core.exceptions import ValidationError
from django.forms.fields import Field, FileField
from django.forms.utils import flatatt, ErrorDict, ErrorList
-from django.forms.widgets import Media, media_property, TextInput, Textarea
+from django.forms.widgets import Media, MediaDefiningClass, TextInput, Textarea
from django.utils.html import conditional_escape, format_html
from django.utils.encoding import smart_text, force_text, python_2_unicode_compatible
from django.utils.safestring import mark_safe
@@ -29,6 +29,7 @@ def pretty_name(name):
return ''
return name.replace('_', ' ').capitalize()
+
def get_declared_fields(bases, attrs, with_base_fields=True):
"""
Create a list of form field instances from the passed in 'attrs', plus any
@@ -40,6 +41,13 @@ def get_declared_fields(bases, attrs, with_base_fields=True):
used. The distinction is useful in ModelForm subclassing.
Also integrates any additional media definitions.
"""
+
+ warnings.warn(
+ "get_declared_fields is deprecated and will be removed in Django 1.9.",
+ PendingDeprecationWarning,
+ stacklevel=2,
+ )
+
fields = [(field_name, attrs.pop(field_name)) for field_name, obj in list(six.iteritems(attrs)) if isinstance(obj, Field)]
fields.sort(key=lambda x: x[1].creation_counter)
@@ -57,19 +65,42 @@ def get_declared_fields(bases, attrs, with_base_fields=True):
return OrderedDict(fields)
-class DeclarativeFieldsMetaclass(type):
+
+class DeclarativeFieldsMetaclass(MediaDefiningClass):
"""
- Metaclass that converts Field attributes to a dictionary called
- 'base_fields', taking into account parent class 'base_fields' as well.
+ Metaclass that collects Fields declared on the base classes.
"""
- def __new__(cls, name, bases, attrs):
- attrs['base_fields'] = get_declared_fields(bases, attrs)
- new_class = super(DeclarativeFieldsMetaclass,
- cls).__new__(cls, name, bases, attrs)
- if 'media' not in attrs:
- new_class.media = media_property(new_class)
+ def __new__(mcs, name, bases, attrs):
+ # Collect fields from current class.
+ current_fields = []
+ for key, value in list(attrs.items()):
+ if isinstance(value, Field):
+ current_fields.append((key, value))
+ attrs.pop(key)
+ current_fields.sort(key=lambda x: x[1].creation_counter)
+ attrs['declared_fields'] = OrderedDict(current_fields)
+
+ new_class = (super(DeclarativeFieldsMetaclass, mcs)
+ .__new__(mcs, name, bases, attrs))
+
+ # Walk through the MRO.
+ declared_fields = OrderedDict()
+ for base in reversed(new_class.__mro__):
+ # Collect fields from base class.
+ if hasattr(base, 'declared_fields'):
+ declared_fields.update(base.declared_fields)
+
+ # Field shadowing.
+ for attr in base.__dict__.keys():
+ if attr in declared_fields:
+ declared_fields.pop(attr)
+
+ new_class.base_fields = declared_fields
+ new_class.declared_fields = declared_fields
+
return new_class
+
@python_2_unicode_compatible
class BaseForm(object):
# This is the main implementation of all the Form logic. Note that this
@@ -398,6 +429,7 @@ def visible_fields(self):
"""
return [field for field in self if not field.is_hidden]
+
class Form(six.with_metaclass(DeclarativeFieldsMetaclass, BaseForm)):
"A collection of Fields, plus their associated data."
# This is a separate class from BaseForm in order to abstract the way
@@ -406,6 +438,7 @@ class Form(six.with_metaclass(DeclarativeFieldsMetaclass, BaseForm)):
# to define a form using declarative syntax.
# BaseForm itself has no way of designating self.fields.
+
@python_2_unicode_compatible
class BoundField(object):
"A Field plus data"
View
48 django/forms/models.py
@@ -10,11 +10,11 @@
from django.core.exceptions import ValidationError, NON_FIELD_ERRORS, FieldError
from django.forms.fields import Field, ChoiceField
-from django.forms.forms import BaseForm, get_declared_fields
+from django.forms.forms import DeclarativeFieldsMetaclass, BaseForm
from django.forms.formsets import BaseFormSet, formset_factory
from django.forms.utils import ErrorList
from django.forms.widgets import (SelectMultiple, HiddenInput,
- MultipleHiddenInput, media_property, CheckboxSelectMultiple)
+ MultipleHiddenInput, CheckboxSelectMultiple)
from django.utils.encoding import smart_text, force_text
from django.utils import six
from django.utils.text import get_text_list, capfirst
@@ -61,6 +61,7 @@ def construct_instance(form, instance, fields=None, exclude=None):
return instance
+
def save_instance(form, instance, fields=None, fail_message='saved',
commit=True, exclude=None, construct=True):
"""
@@ -138,6 +139,7 @@ def model_to_dict(instance, fields=None, exclude=None):
data[f.name] = f.value_from_object(instance)
return data
+
def fields_for_model(model, fields=None, exclude=None, widgets=None,
formfield_callback=None, localized_fields=None,
labels=None, help_texts=None, error_messages=None):
@@ -207,6 +209,7 @@ def fields_for_model(model, fields=None, exclude=None, widgets=None,
)
return field_dict
+
class ModelFormOptions(object):
def __init__(self, options=None):
self.model = getattr(options, 'model', None)
@@ -219,22 +222,16 @@ def __init__(self, options=None):
self.error_messages = getattr(options, 'error_messages', None)
-class ModelFormMetaclass(type):
- def __new__(cls, name, bases, attrs):
+class ModelFormMetaclass(DeclarativeFieldsMetaclass):
+ def __new__(mcs, name, bases, attrs):
formfield_callback = attrs.pop('formfield_callback', None)
- try:
- parents = [b for b in bases if issubclass(b, ModelForm)]
- except NameError:
- # We are defining ModelForm itself.
- parents = None
- declared_fields = get_declared_fields(bases, attrs, False)
- new_class = super(ModelFormMetaclass, cls).__new__(cls, name, bases,
- attrs)
- if not parents:
+
+ new_class = (super(ModelFormMetaclass, mcs)
+ .__new__(mcs, name, bases, attrs))
+
+ if bases == (BaseModelForm,):
return new_class
- if 'media' not in attrs:
- new_class.media = media_property(new_class)
opts = new_class._meta = ModelFormOptions(getattr(new_class, 'Meta', None))
# We check if a string was passed to `fields` or `exclude`,
@@ -253,7 +250,6 @@ def __new__(cls, name, bases, attrs):
if opts.model:
# If a model is defined, extract form fields from it.
-
if opts.fields is None and opts.exclude is None:
# This should be some kind of assertion error once deprecation
# cycle is complete.
@@ -263,7 +259,7 @@ def __new__(cls, name, bases, attrs):
DeprecationWarning, stacklevel=2)
if opts.fields == ALL_FIELDS:
- # sentinel for fields_for_model to indicate "get the list of
+ # Sentinel for fields_for_model to indicate "get the list of
# fields from the model"
opts.fields = None
@@ -274,8 +270,8 @@ def __new__(cls, name, bases, attrs):
# make sure opts.fields doesn't specify an invalid field
none_model_fields = [k for k, v in six.iteritems(fields) if not v]
- missing_fields = set(none_model_fields) - \
- set(declared_fields.keys())
+ missing_fields = (set(none_model_fields) -
+ set(new_class.declared_fields.keys()))
if missing_fields:
message = 'Unknown field(s) (%s) specified for %s'
message = message % (', '.join(missing_fields),
@@ -283,13 +279,15 @@ def __new__(cls, name, bases, attrs):
raise FieldError(message)
# Override default model fields with any custom declared ones
# (plus, include all the other declared fields).
- fields.update(declared_fields)
+ fields.update(new_class.declared_fields)
else:
- fields = declared_fields
- new_class.declared_fields = declared_fields
+ fields = new_class.declared_fields
+
new_class.base_fields = fields
+
return new_class
+
class BaseModelForm(BaseForm):
def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
initial=None, error_class=ErrorList, label_suffix=None,
@@ -438,9 +436,11 @@ def save(self, commit=True):
save.alters_data = True
+
class ModelForm(six.with_metaclass(ModelFormMetaclass, BaseModelForm)):
pass
+
def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
formfield_callback=None, widgets=None, localized_fields=None,
labels=None, help_texts=None, error_messages=None):
@@ -780,6 +780,7 @@ def pk_is_not_editable(pk):
form.fields[self._pk_field.name] = ModelChoiceField(qs, initial=pk_value, required=False, widget=widget)
super(BaseModelFormSet, self).add_fields(form, index)
+
def modelformset_factory(model, form=ModelForm, formfield_callback=None,
formset=BaseModelFormSet, extra=1, can_delete=False,
can_order=False, max_num=None, fields=None, exclude=None,
@@ -1021,6 +1022,7 @@ def clean(self, value):
def _has_changed(self, initial, data):
return False
+
class ModelChoiceIterator(object):
def __init__(self, field):
self.field = field
@@ -1047,6 +1049,7 @@ def __len__(self):
def choice(self, obj):
return (self.field.prepare_value(obj), self.field.label_from_instance(obj))
+
class ModelChoiceField(ChoiceField):
"""A ChoiceField whose choices are a model QuerySet."""
# This class is a subclass of ChoiceField for purity, but it doesn't
@@ -1141,6 +1144,7 @@ def _has_changed(self, initial, data):
data_value = data if data is not None else ''
return force_text(self.prepare_value(initial_value)) != force_text(data_value)
+
class ModelMultipleChoiceField(ModelChoiceField):
"""A MultipleChoiceField whose choices are a model QuerySet."""
widget = SelectMultiple
View
12 django/forms/widgets.py
@@ -131,12 +131,16 @@ def _media(self):
return property(_media)
class MediaDefiningClass(type):
- "Metaclass for classes that can have media definitions"
- def __new__(cls, name, bases, attrs):
- new_class = super(MediaDefiningClass, cls).__new__(cls, name, bases,
- attrs)
+ """
+ Metaclass for classes that can have media definitions.
+ """
+ def __new__(mcs, name, bases, attrs):
+ new_class = (super(MediaDefiningClass, mcs)
+ .__new__(mcs, name, bases, attrs))
+
if 'media' not in attrs:
new_class.media = media_property(new_class)
+
return new_class
@python_2_unicode_compatible
View
2  docs/internals/deprecation.txt
@@ -467,6 +467,8 @@ these changes.
* The ``use_natural_keys`` argument for ``serializers.serialize()`` will be
removed. Use ``use_natural_foreign_keys`` instead.
+* ``django.forms.get_declared_fields`` will be removed.
+
2.0
---
View
7 docs/ref/forms/api.txt
@@ -854,6 +854,13 @@ classes::
<li>Instrument: <input type="text" name="instrument" /></li>
<li>Haircut type: <input type="text" name="haircut_type" /></li>
+.. versionadded:: 1.7
+
+* It's possible to opt-out from a ``Field`` inherited from a parent class by
+ shadowing it. While any non-``Field`` value works for this purpose, it's
+ recommended to use ``None`` to make it explicit that a field is being
+ nullified.
+
.. _form-prefix:
Prefixes for forms
View
14 docs/releases/1.7.txt
@@ -288,6 +288,14 @@ Forms
:func:`~django.forms.formsets.formset_factory` to allow validating
a minimum number of submitted forms.
+* The metaclasses used by ``Form`` and ``ModelForm`` have been reworked to
+ support more inheritance scenarios. The previous limitation that prevented
+ inheriting from both ``Form`` and ``ModelForm`` simultaneously have been
+ removed as long as ``ModelForm`` appears first in the MRO.
+
+* It's now possible to opt-out from a ``Form`` field declared in a parent class
+ by shadowing it with a non-``Field`` value.
+
Internationalization
^^^^^^^^^^^^^^^^^^^^
@@ -513,6 +521,12 @@ Miscellaneous
still worked, even though it was not documented or officially supported. If
you're still using it, please update to the current :setting:`CACHES` syntax.
+* The default ordering of ``Form`` fields in case of inheritance has changed to
+ follow normal Python MRO. Fields are now discovered by iterating through the
+ MRO in reverse with the topmost class coming last. This only affects you if
+ you relied on the default field ordering while having fields defined on both
+ the current class *and* on a parent ``Form``.
+
Features deprecated in 1.7
==========================
View
23 docs/topics/forms/modelforms.txt
@@ -644,11 +644,24 @@ There are a couple of things to note, however.
used. This means the child's ``Meta``, if it exists, otherwise the
``Meta`` of the first parent, etc.
-* For technical reasons, a subclass cannot inherit from both a ``ModelForm``
- and a ``Form`` simultaneously.
+.. versionchanged:: 1.7
-Chances are these notes won't affect you unless you're trying to do something
-tricky with subclassing.
+* It's possible to inherit from both ``Form`` and ``ModelForm`` simultaneuosly,
+ however, you must ensure that ``ModelForm`` appears first in the MRO. This is
+ because these classes rely on different metaclasses and a class can only have
+ one metaclass.
+
+.. versionadded:: 1.7
+
+* It's possible to opt-out from a ``Field`` inherited from a parent class by
+ shadowing it. While any non-``Field`` value works for this purpose, it's
+ recommended to use ``None`` to make it explicit that a field is being
+ nullified.
@mjtamlyn Collaborator

If you add the comment here as discussed, I'm happy to merge this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ You can only use this technique to opt out from a field defined declaratively
+ by a parent class; it won't prevent the ``ModelForm`` metaclass from generating
+ a default field. To opt-out from default fields, see
+ :ref:`controlling-fields-with-fields-and-exclude`.
.. _modelforms-factory:
@@ -748,6 +761,8 @@ instances of the model, you can specify an empty QuerySet::
>>> AuthorFormSet(queryset=Author.objects.none())
+.. _controlling-fields-with-fields-and-exclude:
+
Controlling which fields are used with ``fields`` and ``exclude``
-----------------------------------------------------------------
View
4 tests/forms_tests/tests/test_forms.py
@@ -1302,10 +1302,10 @@ class Beatle(Person, Instrument):
haircut_type = CharField()
b = Beatle(auto_id=False)
- self.assertHTMLEqual(b.as_ul(), """<li>First name: <input type="text" name="first_name" /></li>
+ self.assertHTMLEqual(b.as_ul(), """<li>Instrument: <input type="text" name="instrument" /></li>
+<li>First name: <input type="text" name="first_name" /></li>
<li>Last name: <input type="text" name="last_name" /></li>
<li>Birthday: <input type="text" name="birthday" /></li>
-<li>Instrument: <input type="text" name="instrument" /></li>
<li>Haircut type: <input type="text" name="haircut_type" /></li>""")
def test_forms_with_prefixes(self):
View
36 tests/model_forms/tests.py
@@ -1802,3 +1802,39 @@ def test_multiple_widgets(self):
html = form.as_p()
self.assertInHTML('<ul id="id_status">', html)
self.assertInHTML(dreaded_help_text, html, count=0)
+
+
+class ModelFormInheritanceTests(TestCase):
+ def test_form_subclass_inheritance(self):
+ class Form(forms.Form):
+ age = forms.IntegerField()
+
+ class ModelForm(forms.ModelForm, Form):
+ class Meta:
+ model = Writer
+ fields = '__all__'
+
+ self.assertEqual(list(ModelForm().fields.keys()), ['name', 'age'])
@mjtamlyn Collaborator

You can probably use assertSequenceEqual here which might be a bit nicer.

@loic Collaborator
loic added a note

I tried, it doesn't work on PY3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ def test_field_shadowing(self):
+ class ModelForm(forms.ModelForm):
+ class Meta:
+ model = Writer
+ fields = '__all__'
+
+ class Mixin(object):
+ age = None
+
+ class Form(forms.Form):
+ age = forms.IntegerField()
+
+ class Form2(forms.Form):
+ foo = forms.IntegerField()
+
+ self.assertEqual(list(ModelForm().fields.keys()), ['name'])
+ self.assertEqual(list(type(str('NewForm'), (Mixin, Form), {})().fields.keys()), [])
+ self.assertEqual(list(type(str('NewForm'), (Form2, Mixin, Form), {})().fields.keys()), ['foo'])
+ self.assertEqual(list(type(str('NewForm'), (Mixin, ModelForm, Form), {})().fields.keys()), ['name'])
+ self.assertEqual(list(type(str('NewForm'), (ModelForm, Mixin, Form), {})().fields.keys()), ['name'])
+ self.assertEqual(list(type(str('NewForm'), (ModelForm, Form, Mixin), {})().fields.keys()), ['name', 'age'])
+ self.assertEqual(list(type(str('NewForm'), (ModelForm, Form), {'age': None})().fields.keys()), ['name'])
Something went wrong with that request. Please try again.