#34899 closed Bug (fixed)
Model Field.choices callable support is not actually lazy
Reported by: | Adam Johnson | Owned by: | Nick Pope |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.0 |
Severity: | Release blocker | Keywords: | choices, callable, lazy |
Cc: | Natalia Bidart, Nick Pope | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
#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 (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.
Change History (10)
comment:1 by , 13 months ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 13 months ago
Cc: | added; removed |
---|
comment:3 by , 13 months ago
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.
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.
comment:4 by , 13 months ago
Has patch: | set |
---|---|
Keywords: | choices callable lazy added |
Owner: | changed from | to
Patch needs improvement: | set |
Status: | new → assigned |
comment:5 by , 13 months ago
Patch needs improvement: | unset |
---|
comment:6 by , 13 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Accepting following the reproducer, I'll be able to take a look after `DjangoCon. Adding Nick as cc as well.