#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
-------------------------------------+-------------------------------------
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 <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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/070.6a978b2365c9d4bc52cd94e4f11d9cdb%40djangoproject.com.
