#30931: Cannot override get_FOO_display() in Django 2.2+.
-------------------------------------+-------------------------------------
Reporter: Jim Ouwerkerk | Owner: Carlton
Type: | Gibson
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
> They pass on 2.1 too...
Err. Can I ask you to check again please. We're obviously seeing something
different.
{{{
$ git show --name-only
commit 522af9d6737550ef035a173c08a8276028b68917 (HEAD -> stable/2.1.x,
origin/stable/2.1.x)
...
(django) ~/Documents/Django-Stack/django/tests (stable/2.1.x)$ git diff
diff --git a/django/db/models/fields/__init__.py
b/django/db/models/fields/__init__.py
index 50d22bef0c..38aed6c818 100644
--- a/django/db/models/fields/__init__.py
+++ b/django/db/models/fields/__init__.py
@@ -744,8 +744,9 @@ class Field(RegisterLookupMixin):
if not getattr(cls, self.attname, None):
setattr(cls, self.attname,
DeferredAttribute(self.attname))
if self.choices:
- setattr(cls, 'get_%s_display' % self.name,
- partialmethod(cls._get_FIELD_display, field=self))
+ if not hasattr(cls, 'get_%s_display' % self.name):
+ setattr(cls, 'get_%s_display' % self.name,
+ partialmethod(cls._get_FIELD_display,
field=self))
def get_filter_kwargs_for_object(self, obj):
"""
diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py
index 45f61a0034..31766a2d5c 100644
--- a/tests/model_fields/tests.py
+++ b/tests/model_fields/tests.py
@@ -157,3 +157,25 @@ class GetChoicesTests(SimpleTestCase):
lazy_func = lazy(lambda x: 0 / 0, int) # raises
ZeroDivisionError if evaluated.
f = models.CharField(choices=[(lazy_func('group'), (('a', 'A'),
('b', 'B')))])
self.assertEqual(f.get_choices(include_blank=True)[0], ('',
'---------'))
+
+ def test_overriding_FIELD_display(self):
+ class FooBar(models.Model):
+ foo_bar = models.CharField(choices=[(1, 'foo'), (2, 'bar')])
+
+ def _get_FIELD_display(self, field):
+ if field.attname == 'foo_bar':
+ return "something"
+ return super()._get_FIELD_display(field)
+
+ f = FooBar(foo_bar=1)
+ self.assertEqual(f.get_foo_bar_display(), "something")
+
+ def test_overriding_FIELD_display_2(self):
+ class FooBar2(models.Model):
+ def get_foo_bar_display(self):
+ return 'something'
+
+ foo_bar = models.CharField(choices=[(1, 'foo'), (2, 'bar')])
+
+ f = FooBar2(foo_bar=1)
+ self.assertEqual(f.get_foo_bar_display(), 'something')
(django) ~/Documents/Django-Stack/django/tests (stable/2.1.x)$
./runtests.py model_fields.tests.GetChoicesTests
Testing against Django installed in '/Users/carlton/Documents/Django-
Stack/django/django' with up to 4 processes
Creating test database for alias 'default'...
Creating test database for alias 'other'...
System check identified no issues (0 silenced).
...F.
======================================================================
FAIL: test_overriding_FIELD_display (model_fields.tests.GetChoicesTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/carlton/Documents/Django-
Stack/django/tests/model_fields/tests.py", line 171, in
test_overriding_FIELD_display
self.assertEqual(f.get_foo_bar_display(), "something")
AssertionError: 'foo' != 'something'
- foo
+ something
----------------------------------------------------------------------
Ran 5 tests in 0.004s
FAILED (failures=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
}}}
It's relevant precisely because it's a change in behaviour. ("Regression
from previous version...")
> I don't think that overriding non-public methods could be considered as
the correct way of customization.
OK. In this case I disagree. (That's allowed.) I think it's a rare use-
case, worth documenting, but not worth adding a code path for.
> It already does...
Yes, but more isn't better here.
(You're welcome to think different.)
@Jim: Yes, I hear your sentiment, but, for me, here you're dealing with
special methods added by the metaclass. It's not a normal case of
subclassing.
I would much rather state, plainly, the ''vanilla'' way of achieving the
desired result, even if pointing to an otherwise private method, than add
code to ''magic'' the same thing. Every hook we add back from `Field`
makes the code here marginally more difficult to maintain and adjust.
Again for me, this is niche use-case that doesn't merit that addition.
I'd hope that at least make sense.
--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:15>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/070.f71d8f97859507ca94d9ff0b84864d58%40djangoproject.com.
