[Django] #30931: Cannot override `get_FIELD

Django

unread,

Oct 30, 2019, 10:22:29 AM10/30/19

to django-...@googlegroups.com

#30931: Cannot override `get_FIELD_display` anymore
-------------------------------------+-------------------------------------
Reporter: Jim | Owner: nobody
Ouwerkerk |
Type: | Status: new
Uncategorized |
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I cannot override the `get_FIELD_display` function on models since version
2.2. It works in version 2.1.

Example:
{{{
class FooBar(models.Model):
foo_bar = models.CharField(_("foo"), choices=[(1, 'foo'), (2, 'bar')])

def __str__(self):
return self.get_foo_bar_display() # This returns 'foo' or 'bar'
in 2.2, but 'something' in 2.1

def get_foo_bar_display(self):
return "something"
}}}

What I expect is that I should be able to override this function.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Oct 30, 2019, 10:22:59 AM10/30/19

to django-...@googlegroups.com

#30931: Cannot override `get_FIELD_display` anymore
-------------------------------------+-------------------------------------

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jim Ouwerkerk):

* type: Uncategorized => Bug

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:1>

Django

unread,

Oct 30, 2019, 10:56:56 AM10/30/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.

-------------------------------------+-------------------------------------
Reporter: Jim Ouwerkerk | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 2.2
(models, ORM) |

Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* cc: Sergey Fedoseev, Carlton Gibson (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted

Comment:

Thanks for this report.

Regression in a68ea231012434b522ce45c513d84add516afa60.
Reproduced at 54a7b021125d23a248e70ba17bf8b10bc8619234.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:2>

Django

unread,

Oct 30, 2019, 11:33:33 AM10/30/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
-------------------------------------+-------------------------------------

Reporter: Jim Ouwerkerk | Owner: Carlton
| Gibson
Type: Bug | Status: assigned

Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Carlton Gibson):

* status: new => assigned
* owner: nobody => Carlton Gibson

Comment:

OK, I have a lead on this. Not at all happy about how it looks at first
pass, but I'll a proof of concept PR together for it tomorrow AM.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:3>

Django

unread,

Oct 30, 2019, 1:42:31 PM10/30/19

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sergey Fedoseev):

I don't think it should be marked as blocker since it looks like it was
never supported, because it depends on the order of `attrs` passed in
`ModelBase.__new__()`. So on Django 2.1 and Python 3.7:

{{{
In [1]: import django
...: django.VERSION
In [2]: from django.db import models
...:
...: class FooBar(models.Model):
...: def get_foo_bar_display(self):
...: return "something"
...:
...: foo_bar = models.CharField("foo", choices=[(1, 'foo'), (2,
'bar')])
...:
...: def __str__(self):
...: return self.get_foo_bar_display() # This returns 'foo' or

'bar' in 2.2, but 'something' in 2.1

...:
...: class Meta:
...: app_label = 'test'
...:
...: FooBar(foo_bar=1)
Out[2]: <FooBar: foo>
}}}

Before [https://docs.python.org/3.8/whatsnew/3.6.html#whatsnew36-pep520
Python 3.6] the order of `attrs` wasn't defined at all.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:4>

Django

unread,

Oct 30, 2019, 2:55:35 PM10/30/19

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by felixxm):

Sergey, an example from the ticket description works for me with Django
2.1 and Python 3.6, 3.7 and 3.8.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:5>

Django

unread,

Oct 30, 2019, 3:53:21 PM10/30/19

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sergey Fedoseev):

{{{
In [2]: import django
...: django.VERSION
Out[2]: (2, 1, 13, 'final', 0)

In [3]: import sys
...: sys.version
Out[3]: '3.5.7 (default, Oct 17 2019, 07:04:41) \n[GCC 8.3.0]'

In [4]: from django.db import models
...:
...: class FooBar(models.Model):
...: foo_bar = models.CharField("foo", choices=[(1, 'foo'), (2,
'bar')])
...:
...: def __str__(self):
...: return self.get_foo_bar_display() # This returns 'foo' or

'bar' in 2.2, but 'something' in 2.1

...:

...: def get_foo_bar_display(self):
...: return "something"
...:

...: class Meta:
...: app_label = 'test'
...:
...: FooBar(foo_bar=1)

Out[4]: <FooBar: foo>
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:6>

Django

unread,

Oct 31, 2019, 5:28:06 AM10/31/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
-------------------------------------+-------------------------------------
Reporter: Jim Ouwerkerk | Owner: Carlton

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Carlton Gibson):

* type: Bug => Cleanup/optimization
* component: Database layer (models, ORM) => Documentation
* severity: Release blocker => Normal

Comment:

OK, so there is a behaviour change here, but Sergey is correct that it
does depend on `attr` order, so it's hard to say that this can be said to
ever have been thought of as supported, with the exact example provided.

This example produces the opposite result on 2.1 (even on >=PY36):

{{{
def test_overriding_display_backwards(self):

class FooBar2(models.Model):

def get_foo_bar_display(self):
return "something"

foo_bar = models.CharField("foo", choices=[(1, 'foo'), (2,
'bar')])

f = FooBar2(foo_bar=1)

# This returns 'foo' or 'bar' in both 2.2 and 2.1
self.assertEqual(f.get_foo_bar_display(), "foo")
}}}

Because `get_foo_bar_display()` is defined **before** `foo_bar` is gets
replaced in the the `add_to_class()` step.

Semantically order shouldn't make a difference. Given that it does, I
can't see that we're bound to maintain that behaviour.
(There's a possible **fix** in `Field.contribute_to_class()` but
implementing that just reverses the pass/fail behaviour depending on
order...)

**Rather**, the correct way to implement this **on 2.2+** is:

{{{
def test_overriding_display(self):
class FooBar(models.Model):
foo_bar = models.CharField("foo", 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")
}}}

This is stable for declaration order on version **2.2+**.

This approach requires overriding `_get_FIELD_display()` **before**
declaring fields on 2.1, because otherwise `Model._get_FIELD_display()` is
picked up during `Field.contribute_to_class()`. This ordering dependency
is, ultimately, the same issue that was addressed in
a68ea231012434b522ce45c513d84add516afa60, and the follow-up in #30254.

The behaviour is 2.1 and before was incorrect. Yes, there's a behaviour
change here but it's a bugfix, and all bugfixes are breaking changes if
you're depending on the broken behaviour.

I'm going to downgrade this from Release Blocker accordingly. I'll
reclassify this as a Documentation issue and provide the working example,
as overriding `_get_FIELD_display()` is a legitimate use-case I'd guess.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:7>

Django

unread,

Oct 31, 2019, 6:21:44 AM10/31/19

to django-...@googlegroups.com

#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: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sergey Fedoseev):

Replying to [comment:7 Carlton Gibson]:

> (There's a possible **fix** in `Field.contribute_to_class()` but
implementing that just reverses the pass/fail behaviour depending on
order...)

Doesn't this fix it?

{{{#!python
if not hasattr(cls, 'get_%s_display' % self.name):
setattr(cls, 'get_%s_display' % self.name,
partialmethod(cls._get_FIELD_display, field=self))
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:8>

Django

unread,

Oct 31, 2019, 6:48:02 AM10/31/19

to django-...@googlegroups.com

#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: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Carlton Gibson):

Hi Sergey,

No. That's exactly the "implementing that just reverses the pass/fail
behaviour depending on order..." — two test cases, one with the override
declared before the field, one after, the one fails as it is, the other
fails if you put that guard in... — That's on 2.1, but it's the underlying
issue being exposed: the right approach is overriding
`_get_FIELD_display()`. (Rather than us trying to patch something into
`Field...`.)

[https://github.com/django/django/pull/11999 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:9>

Django

unread,

Oct 31, 2019, 6:48:17 AM10/31/19

to django-...@googlegroups.com

#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
-------------------------------------+-------------------------------------

Changes (by Carlton Gibson):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:10>

Django

unread,

Oct 31, 2019, 7:16:58 AM10/31/19

to django-...@googlegroups.com

#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 Sergey Fedoseev):

Replying to [comment:9 Carlton Gibson]:

> Hi Sergey,
>
> No. That's exactly the "implementing that just reverses the pass/fail
behaviour depending on order..." — two test cases, one with the override
declared before the field, one after, the one fails as it is, the other
fails if you put that guard in... — That's on 2.1, but it's the underlying
issue being exposed: the right approach is overriding
`_get_FIELD_display()`. (Rather than us trying to patch something into
`Field...`.)
>
> [https://github.com/django/django/pull/11999 PR]
>
>
>

[https://github.com/sir-
sigurd/django/commit/2dd6421ff0c29b19f8755e2d5638fc913224bf87 Both tests]
pass for me ¯\_(ツ)_/¯.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:11>

Django

unread,

Oct 31, 2019, 8:02:12 AM10/31/19

to django-...@googlegroups.com

#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):

Yes, on 2.2 they would. But it's a change of behaviour from 2.1 (which is
the point here I take it).

The new `__new__()` behaviour is correct™.

So we have a choice:

* Add code to `Field`, which IMO is very much the wrong place. (Field
shouldn't need to examine the `__dict__` of the class it's being applied
to — that way lies madness, or
* Document the correct way to customize this, requiring 0 code changes.

I've opted for the latter.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:12>

Django

unread,

Oct 31, 2019, 8:58:55 AM10/31/19

to django-...@googlegroups.com

#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 Sergey Fedoseev):

Replying to [comment:12 Carlton Gibson]:

> Yes, on 2.2 they would.

They pass on 2.1 too, since the mentioned commit is based on stable/2.1.x
branch, but I'm not sure why this is relevant since we agreed that this
isn't something that should be backported.

> Field shouldn't need to examine the `__dict__` of the class it's being
applied to

It already
[https://github.com/django/django/blob/3cf907c20c4f4d94f649fbb93a006af5c61b30b8/django/db/models/fields/__init__.py#L763-L764
does it].

> Document the correct way to customize this, requiring 0 code changes.

I don't think that overriding non-public methods could be considered as
the correct way of customization.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:13>

Django

unread,

Oct 31, 2019, 9:54:33 AM10/31/19

to django-...@googlegroups.com

#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 Jim Ouwerkerk):

I would expect that overriding the function should work. It's pretty
common to override functions in Python and dev's expect that they can
override other methods. I don't understand why it should not be supported
for the `get_FOO_display` method. I think it's a basic expectation that it
would work.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:14>

Django

unread,

Oct 31, 2019, 10:35:14 AM10/31/19

to django-...@googlegroups.com

#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

unread,

Oct 31, 2019, 10:36:01 AM10/31/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.

--------------------------------------+------------------------------------
Reporter: Jim Ouwerkerk | Owner: (none)
Type: Cleanup/optimization | Status: new

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

--------------------------------------+------------------------------------
Changes (by Carlton Gibson):

* status: assigned => new
* owner: Carlton Gibson => (none)

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:16>

Django

unread,

Oct 31, 2019, 11:22:27 AM10/31/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
--------------------------------------+------------------------------------
Reporter: Jim Ouwerkerk | Owner: (none)
Type: Cleanup/optimization | Status: new
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 Sergey Fedoseev):

Replying to [comment:15 Carlton Gibson]:

> We're obviously seeing something different.

I guess that's because you override `_get_FIELD_display()` but not
`get_foo_bar_display()` in `test_overriding_FIELD_display`.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:17>

Django

unread,

Oct 31, 2019, 11:38:31 AM10/31/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
--------------------------------------+------------------------------------
Reporter: Jim Ouwerkerk | Owner: (none)
Type: Cleanup/optimization | Status: new
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):

Ah, yes, you're right. I've copied the second test in wrong.

But the point is (still) the change in behaviour from 2.1.

That's still a bug fix, so OK it's changed, that being the nature of bug
fixes.

I'm happy to document this, as suggested. I'd be happy to instead just add
a warning saying "Meh, don't override methods added by the
metaclass....".
Between those, I quite like the option of documenting the solution, since
it opens the box for people a little. The warning just leads to a
"Why...?", and given that it's come up, it's presumably a valid use-case.

Anyhow, I'm leaving to to Mariusz now.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:18>

Django

unread,

Oct 31, 2019, 11:39:46 AM10/31/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
--------------------------------------+------------------------------------
Reporter: Jim Ouwerkerk | Owner: (none)
Type: Cleanup/optimization | Status: new
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

--------------------------------------+------------------------------------
Changes (by Carlton Gibson):

* cc: Carlton Gibson (removed)

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:19>

Django

unread,

Nov 1, 2019, 3:28:35 AM11/1/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
--------------------------------------+------------------------------------
Reporter: Jim Ouwerkerk | Owner: (none)
Type: Cleanup/optimization | Status: new
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 Claude Paroz):

I also think that if we find a way to make the `get_<field>_display()`
properly override the metaclass-added method, I think we should do it,
possibly with a backwards compatibility note if it breaks some old
behavior.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:20>

Django

unread,

Nov 1, 2019, 4:26:35 AM11/1/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
--------------------------------------+------------------------------------
Reporter: Jim Ouwerkerk | Owner: (none)
Type: Cleanup/optimization | Status: new
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):

Then the fix is easy enough. It’s just the addition to
`Field.contribute_to_class()`.

But if we’re going to ''fix'’ it, then it should be as a Release Blocker
to be backported to 2.2. (Otherwise it ≈works in 2.1, doesn’t work in 2.2
and works again in 3.0.)

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:21>

Django

unread,

Nov 1, 2019, 4:33:22 AM11/1/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
--------------------------------------+------------------------------------
Reporter: Jim Ouwerkerk | Owner: (none)
Type: Cleanup/optimization | Status: new
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 Claude Paroz):

I think that's the right thing to do.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:22>

Django

unread,

Nov 1, 2019, 4:47:52 AM11/1/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
--------------------------------------+------------------------------------
Reporter: Jim Ouwerkerk | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 2.2

Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Carlton Gibson):

* severity: Normal => Release blocker

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:23>

Django

unread,

Nov 1, 2019, 4:11:10 PM11/1/19

to django-...@googlegroups.com

#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: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Carlton Gibson):

* status: new => assigned
* owner: (none) => Carlton Gibson

Comment:

PR updated ready for Monday.

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:24>

Django

unread,

Nov 4, 2019, 2:16:14 AM11/4/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
-------------------------------------+-------------------------------------
Reporter: Jim Ouwerkerk | Owner: Carlton
Type: | Gibson

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"2d38eb0ab9f78d68c083a5b78b1eca39027b279a" 2d38eb0a]:
{{{
#!CommitTicketReference repository=""
revision="2d38eb0ab9f78d68c083a5b78b1eca39027b279a"
Fixed #30931 -- Restored ability to override Model.get_FIELD_display().

Thanks Sergey Fedoseev for the implementation idea.

Regression in a68ea231012434b522ce45c513d84add516afa60.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:25>

Django

unread,

Nov 4, 2019, 2:16:16 AM11/4/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
-------------------------------------+-------------------------------------
Reporter: Jim Ouwerkerk | Owner: Carlton
Type: | Gibson
Cleanup/optimization | Status: closed
Component: Documentation | Version: 2.2
Severity: Release blocker | Resolution: fixed
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 Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"dd2ca8b0ea46a56aced70c46dc7283cf0cab5bef" dd2ca8b0]:
{{{
#!CommitTicketReference repository=""
revision="dd2ca8b0ea46a56aced70c46dc7283cf0cab5bef"
[3.0.x] Fixed #30931 -- Restored ability to override
Model.get_FIELD_display().

Thanks Sergey Fedoseev for the implementation idea.

Regression in a68ea231012434b522ce45c513d84add516afa60.

Backport of 2d38eb0ab9f78d68c083a5b78b1eca39027b279a from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:26>

Django

unread,

Nov 4, 2019, 2:19:36 AM11/4/19

to django-...@googlegroups.com

#30931: Cannot override get_FOO_display() in Django 2.2+.
-------------------------------------+-------------------------------------
Reporter: Jim Ouwerkerk | Owner: Carlton
Type: | Gibson
Cleanup/optimization | Status: closed
Component: Documentation | Version: 2.2
Severity: Release blocker | Resolution: fixed
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 Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"785d1706c411168d5acfa709157f60399ea690d0" 785d170]:
{{{
#!CommitTicketReference repository=""
revision="785d1706c411168d5acfa709157f60399ea690d0"
[2.2.x] Fixed #30931 -- Restored ability to override
Model.get_FIELD_display().

Thanks Sergey Fedoseev for the implementation idea.

Regression in a68ea231012434b522ce45c513d84add516afa60.

Backport of 2d38eb0ab9f78d68c083a5b78b1eca39027b279a from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30931#comment:27>

Reply all

Reply to author

Forward

[Django] #30931: Cannot override `get_FIELD_display` anymore

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django