[Django] #34899: Model Field.choices callable support is not actually lazy
Django
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: nobody
Johnson |
Type: Bug | Status: new
Component: Database | Version: 5.0
layer (models, ORM) |
Severity: Release | Keywords:
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
#24561 added support for callables to model Field.choices, with the docs
declaring that using a callable “can be particularly handy” when you need
I/O like a database query. Whilst the model Field supports this by only
lazily calling choices, the formfield() method ends up iterating over
choices, leading to the callable being run at import time with a
`ModelField` definition.
Here’s a minimal example ([https://github.com/adamchainz/django-5.0
-choices-laziness source]):
{{{
from django import forms
from django.db import models
ready = False
def animals():
if not ready:
raise RuntimeError("Not ready to load animals")
return [
(1, "Aardvark"),
(2, "Banana"),
]
class User(models.Model):
spirit_animal = models.IntegerField(choices=animals)
class UserForm(forms.ModelForm):
class Meta:
model = User
fields = ["spirit_animal"]
ready = True
}}}
On Django 5.0a1 it fails with:
{{{
$ ./manage.py shell
Traceback (most recent call last):
File "/..././manage.py", line 21, in <module>
main()
...
File "/.../example/models.py", line 21, in <module>
class UserForm(forms.ModelForm):
File "/.../.venv/lib/python3.11/site-packages/django/forms/models.py",
line 309, in __new__
fields = fields_for_model(
^^^^^^^^^^^^^^^^^
File "/.../.venv/lib/python3.11/site-packages/django/forms/models.py",
line 234, in fields_for_model
formfield = f.formfield(**kwargs)
^^^^^^^^^^^^^^^^^^^^^
File "/.../.venv/lib/python3.11/site-
packages/django/db/models/fields/__init__.py", line 2142, in formfield
return super().formfield(
^^^^^^^^^^^^^^^^^^
File "/.../.venv/lib/python3.11/site-
packages/django/db/models/fields/__init__.py", line 1118, in formfield
defaults["choices"] = self.get_choices(include_blank=include_blank)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/.../.venv/lib/python3.11/site-
packages/django/db/models/fields/__init__.py", line 1054, in get_choices
choices = list(self.choices)
^^^^^^^^^^^^^^^^^^
File "/.../.venv/lib/python3.11/site-packages/django/utils/choices.py",
line 17, in __iter__
yield from normalize_choices(self.func())
^^^^^^^^^^^
File "/.../example/models.py", line 9, in animals
raise RuntimeError("Not ready to load animals")
RuntimeError: Not ready to load animals
}}}
I encountered this bug whilst trying to update django-countries to support
Django 5.0 (https://github.com/SmileyChris/django-countries/pull/438). Its
sorted, translated list of countries is exactly what the callable choices
feature is intended for.
--
Ticket URL: <https://code.djangoproject.com/ticket/34899>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 5.0
(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
-------------------------------------+-------------------------------------
* cc: ngpope (added)
* stage: Unreviewed => Accepted
Comment:
Accepting following the reproducer, I'll be able to take a look after
`DjangoCon. Adding Nick as cc as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 5.0
(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 Natalia Bidart):
* cc: ngpope (removed)
* cc: Nick Pope (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 5.0
(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
-------------------------------------+-------------------------------------
Comment (by apoorvaeternity):
Here
[https://github.com/django/django/blob/f6629ee2c986d3bf59b4c1b3058f370a00bdc573/django/db/models/fields/__init__.py#L1115]
if we add a check for `CallableChoiceIterator` and set
`defaults["choices"]` as the `choices callable` instead of calling the
`get_choices` method directly, that seems to fix this issue.
{{{#!python
if isinstance(self.choices, CallableChoiceIterator):
defaults["choices"] = self.choices
else:
include_blank = self.blank or not (
self.has_default() or "initial" in kwargs
)
defaults["choices"] = self.get_choices(include_blank=include_blank)
}}}
The only caveat with this solution is that the blank choice has to be set
within the `choices callable` and would no longer be handled by
`get_choices` as it relies on checking if a blank choice exists as one of
the available choices which itself requires calling the `choices
callable`.
I have tested this using the example posted in the issue description and
it works. If this solution looks good I would be happy to land a PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
lazy |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* keywords: => choices, callable, lazy
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* status: new => assigned
* owner: nobody => Nick Pope
Comment:
[https://github.com/django/django/pull/17370 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: choices, callable, | Triage Stage: Accepted
lazy |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Nick Pope):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
lazy | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: choices, callable, | Triage Stage: Ready for
lazy | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):
In [changeset:"74afcee234f8be989623ccc7c28b9fb97fb548f0" 74afcee]:
{{{
#!CommitTicketReference repository=""
revision="74afcee234f8be989623ccc7c28b9fb97fb548f0"
Refs #34899 -- Extracted Field.flatchoices to flatten_choices helper
function.
Co-authored-by: Natalia Bidart <124304+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Nick Pope
Component: Database layer | Version: 5.0
(models, ORM) |
Keywords: choices, callable, | Triage Stage: Ready for
lazy | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"171f91d9ef5177850c2f12b26dd732785f6ac034" 171f91d]:
{{{
#!CommitTicketReference repository=""
revision="171f91d9ef5177850c2f12b26dd732785f6ac034"
Fixed #34899 -- Added blank choice to forms' callable choices lazily.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Nick Pope
Type: Bug | Status: closed
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: choices, callable, | Triage Stage: Ready for
lazy | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):
In [changeset:"bbe90f3c00eb29a8e86b1b638466029def7f444a" bbe90f3]:
{{{
#!CommitTicketReference repository=""
revision="bbe90f3c00eb29a8e86b1b638466029def7f444a"
[5.0.x] Refs #34899 -- Extracted Field.flatchoices to flatten_choices
helper function.
Co-authored-by: Natalia Bidart <124304+...@users.noreply.github.com>
Backport of 74afcee234f8be989623ccc7c28b9fb97fb548f0 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Nick Pope
Type: Bug | Status: closed
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: choices, callable, | Triage Stage: Ready for
lazy | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):
In [changeset:"cc5901fa8edc25ce6d67d110c18ddf9f16965e32" cc5901f]:
{{{
#!CommitTicketReference repository=""
revision="cc5901fa8edc25ce6d67d110c18ddf9f16965e32"
[5.0.x] Fixed #34899 -- Added blank choice to forms' callable choices
lazily.
Backport of 171f91d9ef5177850c2f12b26dd732785f6ac034 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34899#comment:10>