[Django] #34983: Deprecate django.utils.itercompat.is_iterable().
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Keywords: itercompat,
Triage Stage: | is_iterable
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
While reviewing the [https://github.com/django/django/pull/17368 PR] for
#10941, I saw that we are still using the
`django.utils.itercompat.is_iterable()` helper function.
It seems that this was originally added in
ad077ccbc03f11c3db3951aab110ec98c5907729 to fix #5445 back in 2007. This
was required because Jython 2.2 didn't have support for `__iter__()`.
Django's support for Jython was removed in 2017 in #28954.
As pointed out in
[https://github.com/django/django/pull/17368#discussion_r1372842813 this
comment], `is_iterable()` also handles the older fallback to determining
if something is iterable by the presence of `__getitem__()` as mentioned
in Python's
[https://docs.python.org/3.12/library/collections.abc.html#id19
documentation]:
> Checking `isinstance(obj, Iterable)` detects classes that are registered
as `Iterable` or that have an `__iter__()` method, but it does not detect
classes that iterate with the `__getitem__()` method. The only reliable
way to determine whether an object is iterable is to call `iter(obj)`.
In practice, however, it is rare that iterables do not support
`__iter__()` and, for all uses of this internal function, it doesn't seem
to be necessary.
I propose that we replace this function with the more standard
`isinstance(..., collections.abc.Iterable)` and deprecate it. ''(Although
the function is private and undocumented, it is likely used in the wild
and other past functions from this module have been through a deprecation
process.)''
--
Ticket URL: <https://code.djangoproject.com/ticket/34983>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Keywords: itercompat, | Triage Stage:
is_iterable | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/17498 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: itercompat, | Triage Stage:
is_iterable | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Thanks for the ticket. Have you checked how it looks like for PyPy? I'm
afraid removing it will make things worse for PyPy. For example
`ChoiceIterator`s don't use `is_iterable()` and they seem to be broken on
PyPy:
{{{
./runtests.py model_fields
....
django.core.management.base.SystemCheckError: SystemCheckError: System
check identified some issues:
ERRORS:
model_fields.Choiceful.choices_from_callable: (fields.E004) 'choices' must
be a mapping (e.g. a dictionary) or an iterable (e.g. a list or tuple).
model_fields.Choiceful.with_choices_dict: (fields.E005) 'choices' must be
a mapping of actual values to human readable names or an iterable
containing (actual value, human readable name) tuples.
model_fields.Choiceful.with_choices_nested_dict: (fields.E005) 'choices'
must be a mapping of actual values to human readable names or an iterable
containing (actual value, human readable name) tuples.
model_fields.Whiz.c: (fields.E005) 'choices' must be a mapping of actual
values to human readable names or an iterable containing (actual value,
human readable name) tuples.
model_fields.WhizDelayed.c: (fields.E005) 'choices' must be a mapping of
actual values to human readable names or an iterable containing (actual
value, human readable name) tuples.
model_fields.WhizIter.c: (fields.E005) 'choices' must be a mapping of
actual values to human readable names or an iterable containing (actual
value, human readable name) tuples.
System check identified 6 issues (0 silenced).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: itercompat, | Triage Stage:
is_iterable | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nick Pope):
I'll dig into the issues around the choices stuff some more, but in the
general simple case it seems like it should be fine:
{{{
❯ pypy3
Python 3.10.13 (f1607341da97ff5a1e93430b6e8c4af0ad1aa019, Oct 02 2023,
19:25:21)
[PyPy 7.3.13 with GCC 13.2.1 20230801] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>>> from collections.abc import Iterable
>>>> class A:
.... def __iter__(self):
.... pass
....
>>>> isinstance(A(), Iterable)
True
>>>>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: itercompat, | Triage Stage:
is_iterable | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
It seems that 500e01073adda32d5149624ee9a5cb7aa3d3583f introduces a big
regression on PyPy, e.g.
- before we had 3 failures in `forms_tests`, after `failures=13,
errors=32`,
- before we had no failures in `model_fields`, after system check
identifies issues, etc.
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: itercompat, | Triage Stage:
is_iterable | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Carsten Fuchs (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: itercompat, | Triage Stage:
is_iterable | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia Bidart):
Replying to [comment:4 Mariusz Felisiak]:
> It seems that 500e01073adda32d5149624ee9a5cb7aa3d3583f introduces a big
regression on PyPy, e.g.
While doing the review for that PR, I did check PyPy compatibility. Is
that something we should be doing? Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: itercompat, | Triage Stage:
is_iterable | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nick Pope):
So 3.10 support in PyPy is still quite new - less than 6 months old.
It looks like we've found a bug for which I've opened an issue:
https://foss.heptapod.net/pypy/pypy/-/issues/4036
That is not related to this ticket, however - what we're looking at here
seems to work fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
is_iterable |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Tentatively accepted.
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: itercompat, | Triage Stage: Accepted
is_iterable |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: itercompat, | Triage Stage: Accepted
is_iterable |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
is_iterable | 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/34983#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Cleanup/optimization |
Component: Utilities | Version: dev
Keywords: itercompat, | Triage Stage: Ready for
is_iterable | 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:"5e28cd3f2cfc31bf947a747256bc036f8f64888a" 5e28cd3f]:
{{{
#!CommitTicketReference repository=""
revision="5e28cd3f2cfc31bf947a747256bc036f8f64888a"
Fixed #34983 -- Deprecated django.utils.itercompat.is_iterable().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: itercompat, | Triage Stage: Ready for
is_iterable | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In [changeset:"f3a2509a54e0b5513199efca5e41a5183fc097f0" f3a2509a]:
{{{#!CommitTicketReference repository=""
revision="f3a2509a54e0b5513199efca5e41a5183fc097f0"
Refs #34983 -- Removed django.utils.itercompat per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34983#comment:13>