classification
Title: zip() shadows TypeError raised in __iter__() of source iterable
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: josh.r, rhettinger, serhiy.storchaka, sir-sigurd, tim.peters
Priority: low Keywords: patch

Created on 2019-08-29 10:30 by sir-sigurd, last changed 2019-08-30 06:23 by rhettinger. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15592 merged sir-sigurd, 2019-08-29 10:31
PR 15607 closed miss-islington, 2019-08-30 04:26
PR 15608 merged miss-islington, 2019-08-30 05:25
Messages (14)
msg350763 - (view) Author: Sergey Fedoseev (sir-sigurd) * Date: 2019-08-29 10:30
zip() shadows TypeError raised in __iter__() of source iterable:

In [21]: class Iterable:
    ...:     def __init__(self, n):
    ...:         self.n = n
    ...:     def __iter__(self):
    ...:         return iter(range(self.n))
    ...:     

In [22]: zip(Iterable('one'))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-22-66835be83afa> in <module>()
----> 1 zip(Iterable('one'))

TypeError: zip argument #1 must support iteration
msg350768 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-29 12:37
I am not even sure this is a bug.
msg350774 - (view) Author: Sergey Fedoseev (sir-sigurd) * Date: 2019-08-29 13:12
Maybe it's not clear from description, but traceback only show the line with zip(), so it doesn't help at localizing the source of exception at all.

You only see that 'argument #N must support iteration', but that argument  has __iter__() i.e. it supports iteration.
msg350775 - (view) Author: Sergey Fedoseev (sir-sigurd) * Date: 2019-08-29 13:25
Also using this example class:

In [5]: iter(Iterable('one'))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-b250104a560e> in <module>()
----> 1 iter(Iterable('one'))

<ipython-input-3-0f0fff743d6c> in __iter__(self)
      3         self.n = n
      4     def __iter__(self):
----> 5         return iter(range(self.n))
      6 

TypeError: range() integer end argument expected, got str.
msg350807 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-29 17:04
> I am not even sure this is a bug.

It really isn't :-)

But it is okay to want to improve the traceback.

Using _PyErr_FormatFromCause() is plausible, but the resulting traceback is messy.  Since there isn't much value in reporting which iterable number has failed, I prefer to just simplify the code with:

-            if (PyErr_ExceptionMatches(PyExc_TypeError))
-                PyErr_Format(PyExc_TypeError,
-                    "zip argument #%zd must support iteration",
-                    i+1);

The same change can also be applied to itertools.zip_longest().
msg350810 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-29 17:49
Agree. This is an improvement of UX.

The initial code was added in 8572b4fedf7e6ee4cd350680d53cd0a21574b083.

Other option is to check ahead if the argument is an iterable.

    if (!item->ob_type->tp_iter && !PySequence_Check(item)) {
        // Raise specific TypeError
    }

(We can introduce a special API for this, PyIterable_Check, if there are other use cases).

This is a part of larger issue. When we parse arguments using Argument Clinic, in some cases we can check the type ahead and report what argument (by name or positional number) of what function has wrong type. But in other cases we just call the converting function, and the detailed information about argument and function is lost. For example:

>>> ''.encode(123)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: encode() argument 'encoding' must be str, not int
>>> ''.encode('\0')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: embedded null character
msg350812 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-29 18:33
> Other option is to check ahead if the argument is an iterable.

The other tools that consume iterators don't go down this path and it doesn't seem to have been a problem in practice.  So, I recommend that we not invent new problems that we didn't already have ;-)

Leaving for Sergey to update his PR.
msg350832 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-29 21:39
I am fine with simplifying the code as Raymond suggested.

map() does not have anything special.
msg350846 - (view) Author: Sergey Fedoseev (sir-sigurd) * Date: 2019-08-30 03:37
> map() does not have anything special.

Just for the reference, in Python 2.7 map() has the same behavior and it caused many problems for me and other people working with/developing Django.
msg350847 - (view) Author: Josh Rosenberg (josh.r) * (Python triager) Date: 2019-08-30 03:42
Raymond: "Since there isn't much value in reporting which iterable number has failed"

Isn't there though? If the error just points to the line with the zip, and the zip is zipping multiple similar things (especially things which won't have a traceable line of Python code associated with them to narrow it down), knowing which argument was the cause of the TypeError seems rather useful. Without it, you just know *something* being zipped was wrong, but need to manually track down which of the arguments was the problem.
msg350848 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-30 03:54
IMO, the index number was never actually helpful.  Also, the new error message takes you straight to the heart of the problem without a useless distractor:

>>> zip('abc', Iterable('one'), 'def')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/raymond/Documents/tmp35.py", line 5, in __iter__
    return iter(range(self.n))
TypeError: 'str' object cannot be interpreted as an integer
msg350850 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-30 04:25
New changeset 6a650aaf7735e30636db2721247f317064c2cfd4 by Raymond Hettinger (Sergey Fedoseev) in branch 'master':
bpo-37976: Prevent shadowing of TypeError in zip() (GH-15592)
https://github.com/python/cpython/commit/6a650aaf7735e30636db2721247f317064c2cfd4
msg350851 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-30 04:27
Thanks Sergey.
msg350856 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-30 06:23
New changeset 27f418640cf39c035114f29cc2d628775b43c0f9 by Raymond Hettinger (Miss Islington (bot)) in branch '3.8':
bpo-37976: Prevent shadowing of TypeError in zip() (GH-15592) (GH-15608)
https://github.com/python/cpython/commit/27f418640cf39c035114f29cc2d628775b43c0f9
History
Date User Action Args
2019-08-30 06:23:28rhettingersetmessages: + msg350856
2019-08-30 05:25:23miss-islingtonsetpull_requests: + pull_request15282
2019-08-30 04:27:11rhettingersetstatus: open -> closed
versions: + Python 3.8
messages: + msg350851

resolution: fixed
stage: patch review -> resolved
2019-08-30 04:26:02miss-islingtonsetpull_requests: + pull_request15281
2019-08-30 04:25:51rhettingersetmessages: + msg350850
2019-08-30 03:54:35rhettingersetmessages: + msg350848
2019-08-30 03:42:32josh.rsetnosy: + josh.r
messages: + msg350847
2019-08-30 03:37:13sir-sigurdsetmessages: + msg350846
2019-08-29 21:39:15serhiy.storchakasetmessages: + msg350832
2019-08-29 18:33:21rhettingersetmessages: + msg350812
2019-08-29 17:49:17serhiy.storchakasetnosy: + tim.peters
type: enhancement
messages: + msg350810
2019-08-29 17:04:17rhettingersetpriority: normal -> low
versions: + Python 3.9
messages: + msg350807

assignee: rhettinger
components: + Library (Lib)
2019-08-29 13:25:26sir-sigurdsetmessages: + msg350775
2019-08-29 13:12:43sir-sigurdsetmessages: + msg350774
2019-08-29 12:37:01serhiy.storchakasetnosy: + rhettinger, serhiy.storchaka
messages: + msg350768
2019-08-29 10:31:11sir-sigurdsetkeywords: + patch
stage: patch review
pull_requests: + pull_request15268
2019-08-29 10:30:10sir-sigurdcreate