Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added support for Django 5.0. #1607

Merged
merged 1 commit into from
Sep 4, 2023
Merged

Added support for Django 5.0. #1607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions django_filters/fields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
RangeWidget,
)

try:
from django.utils.choices import BaseChoiceIterator, normalize_choices
except ImportError:
DJANGO_50 = False
else:
DJANGO_50 = True


class RangeField(forms.MultiValueField):
widget = RangeWidget
Expand Down Expand Up @@ -210,7 +217,7 @@ def clean(self, value):
return value


class ChoiceIterator:
class ChoiceIterator(BaseChoiceIterator if DJANGO_50 else object):
# Emulates the behavior of ModelChoiceIterator, but instead wraps
# the field's _choices iterable.

Expand All @@ -223,7 +230,10 @@ def __iter__(self):
yield ("", self.field.empty_label)
if self.field.null_label is not None:
yield (self.field.null_value, self.field.null_label)
yield from self.choices
if DJANGO_50:
yield from normalize_choices(self.choices)
carltongibson marked this conversation as resolved.
Show resolved Hide resolved
else:
yield from self.choices

def __len__(self):
add = 1 if self.field.empty_label is not None else 0
Expand Down Expand Up @@ -257,16 +267,21 @@ def __init__(self, *args, **kwargs):

super().__init__(*args, **kwargs)

def _get_choices(self):
return super()._get_choices()

def _set_choices(self, value):
super()._set_choices(value)
value = self.iterator(self, self._choices)

self._choices = self.widget.choices = value

choices = property(_get_choices, _set_choices)
@property
def choices(self):
return super().choices
carltongibson marked this conversation as resolved.
Show resolved Hide resolved

@choices.setter
def choices(self, value):
if DJANGO_50:
value = self.iterator(self, value)
else:
super()._set_choices(value)
value = self.iterator(self, self._choices)
Comment on lines +279 to +280
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This double setting case is required in Django < 5.0 because callables are handled very specifically whereas it's all handled in django.utils.choices.normalize_choices() for Django 5.0.


# Simple `super()` syntax for calling a parent property setter is
# unsupported. See https://github.com/python/cpython/issues/59170
super(ChoiceIteratorMixin, self.__class__).choices.__set__(self, value)
Comment on lines +282 to +284
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most unfortunate bit, but works well nonetheless.

Ideally this whole override ought to be as simple as super().choices = self.iterator(self, value).

But currently there is no support for calling the setter of a parent property, see python/cpython#59170, although there does seem to be indication that a fix for this would be acceptable, see python/cpython#29950 (comment).

There is this workaround which is even used in Python itself but there were challenges observed regarding mixins. I have managed to make this work for this mixin by using super(ChoiceIteratorMixin, self.__class__) instead of super(self.__class__, self.__class__) which ensures that when calling e.g. ChoiceField().choices = (for a subclass of the mixin), the super() call will not be passed ChoiceField as the value for self.__class__ as the first argument. Passing ChoiceIteratorMixin means that the next lookup in the MRO will be for forms.ChoiceField.choices.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I should also mention that this only works if we're using @choices.setter here and the property is defined here, not in the parent otherwise we end up with infinite recursion. Another reason why the getter must be redefined here.)



# Unlike their Model* counterparts, forms.ChoiceField and forms.MultipleChoiceField do not set empty_label
Expand Down
3 changes: 0 additions & 3 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,3 @@ commands = python -Werror ./runtests.py --testrunner xmlrunner.extra.djangotestr
deps =
{[latest]deps}
-rrequirements/test-ci.txt

[testenv:{py310, py311}-latest]
ignore_outcome = True