This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Function calls taking a generator as star argument can mask TypeErrors in the generator
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: DragonFireCK, Michel Desmoulin, Samuel BOVÉE, amaury.forgeotdarc, benjamin.peterson, brandjon, dangyogi, daniel.urban, georg.brandl, gpolo, hagen, jaraco, kcarnold, martin.panter, ncoghlan, pfctdayelise, pitrou, python-dev, r.david.murray, ron_adam, terry.reedy, yselivanov
Priority: normal Keywords: needs review, patch

Created on 2009-01-02 12:46 by hagen, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
message_and_docs.patch hagen, 2009-06-03 09:14 Changes error message and docs
TypeError2.patch hagen, 2009-06-03 11:46 Correct masking of TypeError in iterables
combined.patch hagen, 2010-08-05 19:13 combined and updated patch
issue4806.patch daniel.urban, 2011-03-07 07:21 review
issue4806-star-TypeError.v2.patch martin.panter, 2015-02-24 04:29 review
check-generator-py2.patch martin.panter, 2016-01-30 11:21 Only handle generator in 2.7 review
Messages (30)
msg78788 - (view) Author: Hagen Fürstenau (hagen) Date: 2009-01-02 12:46
If we call some function f with a generator as star argument and this
generator raises a TypeError, we get the following exception:

>>> def f(x): pass
... 
>>> def broken(): raise TypeError
... 
>>> f(*(broken() for x in (0,)))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: f() argument after * must be a sequence, not generator

This is a misleading error message, as it's usually no problem to use a
generator as a star argument. Even just replacing the TypeError by some
other exception leads to the expected result, i.e. the exception gets
correctly propagated.

The problem seems to be around line 3710 of Python/ceval.c where the
generator is converted to a tuple. If this conversion raises a
TypeError, then the error message is replaced, which will mask any
TypeError raised by the generator.

I'm not sure how to solve this. We probably can't distinguish a "good"
TypeError from a "bad" TypeError at this point, so we might have to make
a special case for the conversion of generators.
msg78800 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-01-02 14:26
The issue1615 has the same kind of problems: an AttributeError is masked
by another one.
msg78957 - (view) Author: Hagen Fürstenau (hagen) Date: 2009-01-03 13:44
I'm getting confused about whether it's actually desired behaviour that
generators can be star arguments.

The error message seems to say it's not: "argument after * must be a
sequence". The docs seem to agree: "If the syntax *expression appears in
the function call, expression must evaluate to a sequence." However
test_extcall specifically tests function calls with (non-sequence)
iterables as star arguments.
msg88770 - (view) Author: Kenneth Arnold (kcarnold) Date: 2009-06-02 22:43
I can confirm that (a) this exact behavior is happening and (b) it quite
confused me (most of the time it works!). What would be a "good"
TypeError? I'd vote for generators to be explicitly supported even if it
required a special case. Thanks!
msg88800 - (view) Author: Hagen Fürstenau (hagen) Date: 2009-06-03 09:13
I added a simple check for iterables. This is not very elegant, but
performance is only affected in the case of an exception. Patch and
corresponsing test are attached as "TypeError.patch".

As I pointed out above, the actual error message "must be a sequence" is
also inconsistent with the implementation (and tests) which allows any
kind of iterable. The attached and independent patch
"message_and_docs.patch" changes this to "must be an iterable" and
corrects docs and tests accordingly.
msg88803 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-06-03 11:10
This patch leaks a reference on each call to PyObject_GetIter(). And I'm
not sure it is a good idea to call this function again: it may be very
expensive! I'd prefer a simple check on the tp_iter member.
msg88807 - (view) Author: Hagen Fürstenau (hagen) Date: 2009-06-03 11:46
Sorry, I had meant to use PyIter_Check instead of PyObject_GetIter.
Don't know why I didn't do so... ;-)

I corrected the patch.
msg104861 - (view) Author: Bruce Frederiksen (dangyogi) Date: 2010-05-03 17:51
I have also hit this error.  I went to report it but found it already entered (good news), but not resolved from nearly a year ago (bad news).

The error masked another bug that I had in my program and it took me quite awhile to figure out what the real problem was.

I use *generator arguments quite a lot, so was surprised to see the error.  So I, for one, can say that if you disable *generator arguments, you will break existing code.

If anybody cares, I have verified that this error also appears in Python2.5 and Python2.4 and am attempting to add python2.5 to the Versions list.  (And yes, *generators were allowed in Python2.4!)

Is this headed for resolution?  Progress on it seems to have stalled nearly a year ago.  Can I vote to revive this?
msg112896 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-08-04 21:08
I verified with 3.1 the two OP cases and that generators work fine as long as they supply the correct number of values.

def f(x): return x
def broken(): return 1
print(f(*(broken() for x in (0,))))
# prints 1

Change (0,) to (0,1) the normal arg num mismatch message appears.

test_extcall tests version of Nothing() that follow both the old and new iteration protocol. It is possible that 'sequence' in meant in the broader sense of finite iterable rather that the narrow sense of
5.6. Sequence Types — str, bytes, bytearray, list, tuple, range

Since that is confusing, I would replace 'sequence' with 'finite iterable'. (Infinite iterables, obviously, are bad, just as in any other uncontrolled situation, such as "a,*b = itertools.count()".)

So, combine the correction and the suggestion above with original and diff against current trunk (py3k branch) if you can or at least 3.1.2.
msg112960 - (view) Author: Hagen Fürstenau (hagen) Date: 2010-08-05 08:55
IIUC, the only change you suggest for my patch is using "finite iterable" instead of "sequence" or "iterable", right?

I've looked at the docs and there seems to be no precedent for "finite iterable". I think it's just as obvious that the iterable has to yield a correct (and finite) number of parameters as the fact that "list(itertools.count())" is a bad idea. So for consistency I would like to keep "iterable".
msg112998 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-08-05 16:34
Ok, leave iterable as is. More important, you have two disjoint patches that I believe should be combined.
msg113022 - (view) Author: Hagen Fürstenau (hagen) Date: 2010-08-05 19:13
Attaching a combined patch against the current py3k.
msg130168 - (view) Author: Daniel Urban (daniel.urban) * (Python triager) Date: 2011-03-06 12:35
I think the patch isn't entirely correct.  It uses PyIter_Check for detecting the case when an *iterable* raises TypeError, but that function actually checks for an *iterator*.  The check on the tp_iter member mentioned by Amaury Forgeot d'Arc probably would be better, but even that wouldn't detect every iterable: "Its presence normally signals that the instances of this type are iterable (although sequences may be iterable without this function)." (http://docs.python.org/dev/py3k/c-api/typeobj.html#PyTypeObject.tp_iter)  (Apparently any object with a __getitem__ is iterable.  By the way, collections.abc.Iterable also doesn't detect this case.)
msg130232 - (view) Author: Daniel Urban (daniel.urban) * (Python triager) Date: 2011-03-07 07:21
I'm attaching an updated patch. Instead !PyIter_Check() this patch checks for tp_iter == NULL && !PySequence_Check.  If this condition is false, PyObject_GetIter has a chance to succeed (and if it fails, we shouldn't mask the exception).  I also added more tests which show why the previous patch was incorrect.
msg134821 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-04-29 23:04
#11944 is probably a duplicate of this and should be checked when this is fixed
msg151110 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2012-01-12 04:51
I haven’t tried to understand what the patches do, but Issue 5218 looks like a very similar problem with a patch including a test case.
msg152580 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-02-04 02:13
#13904 is another dup, with patches to test and ceval. I asked that they be reloaded to this issue.
msg152583 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-02-04 02:28
#5218 is a 4th duplicate, also with a patch to the ceval loop and test.
msg152584 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-02-04 02:31
Sorry Martin, I see you already said that. Anyway, I closed other three in favor of this one.
msg214091 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-19 12:54
Sounds like we just need someone comfortable with modifying ceval.c to apply this ;)
msg236475 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-02-24 04:29
I am posting a new version of Daniel’s patch as issue4806-star-TypeError.v2.patch:

* Merged with recent “default” (3.5) branch
* Dropped the documentation fix, since revision a8aa918041c2 already fixed that in an identical fashion
* Changed to a “look before you leap” style check before attempting PySequence_Tuple(). This way we get to keep other valid errors such as “iter() returned non-iterator”.

I am comfortable with the code changes previous patch as well as mine. Checking the “tp_iter” field and PySequence_Check() is basically what happens behind the scenes, via PyObject_GetIter() anyway. So I think either patch is valid and a worthwhile improvement.
msg258688 - (view) Author: Samuel BOVÉE (Samuel BOVÉE) Date: 2016-01-20 15:59
Up for this bug. I got this bug in my code and loose one hour to understand what happened. Please review Martin's patch !
msg259239 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-01-30 06:18
I think this is ready to push for Python 3. Python 2 might need an extra Py_TPFLAGS_HAVE_ITER check, perhaps reusing some code from the Issue 5218 patch.
msg259240 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-01-30 06:47
New changeset 1a8dc350962b by Martin Panter in branch '3.5':
Issue #4806: Avoid masking original TypeError in call with * unpacking
https://hg.python.org/cpython/rev/1a8dc350962b

New changeset 1261f5f6fc95 by Martin Panter in branch 'default':
Issue #4806: Merge * unpacking fix from 3.5
https://hg.python.org/cpython/rev/1261f5f6fc95
msg259251 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-01-30 11:21
In Python 2, the old-style “instance” objects are making it hard to fix this like in Python 3. And completely removing the custom error message seems a bit drastic for 2.7.

Instead, I propose to just check for a generator type after the TypeError is caught, and don’t bother checking for other kinds of iterables and sequences.
msg260300 - (view) Author: Michel Desmoulin (Michel Desmoulin) Date: 2016-02-15 08:34
It gets even weirder with coroutines involved:



>>> f = (coro() for coro in l) 
>>> asyncio.gather(*f) # TypeError: asyncio.gather() argument after * must be a sequence, not generator
>>> asyncio.gather(*[*f]) # ok

Because coroutines are executed later, the last gather() doesn't show the error message. So it tricked you into thinking that the second version is ok while the first one is not.

But of course, both are ok, the problem lies in a bug deeper in your coroutine triggering the TypeError.
msg260302 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-15 09:04
Michel: I suspect your code doesn’t actually get up to handling any coroutines, and the problem is in your generator expression. What is “l”, and what are the items in it? The bug should already be fixed ready for the 3.5.2 and 3.6 releases, but I can produce this on 3.5.0:

>>> l = [None]
>>> f = (coro() for coro in l) 
>>> asyncio.gather(*f)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: gather() argument after * must be a sequence, not generator

In the current 3.6 code the bug is fixed:

>>> asyncio.gather(*f)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 1, in <genexpr>
TypeError: 'NoneType' object is not callable

The reason why the second call does nothing interesting is because the generator expression has already die because of the exception, and there is nothing left to iterate:

>>> remaining = [*f]
>>> remaining
[]
>>> asyncio.gather(*remaining)
<Future finished result=[]>
msg260304 - (view) Author: Michel Desmoulin (Michel Desmoulin) Date: 2016-02-15 09:42
We fixed our bug days ago, but I would have expected [*gen] to have triggered an exception before it even got to gather().

The real code was something like:

>>> l = (ensure_awaitable(callable_obj) for callable_obj in callable_list)
>>> gather(*l)

ensure_awaitable() is just using inspect to turn coroutine functions into coroutines, and wraps non coroutine callables with asyncio.coroutine(callable)().

In the end, ensure_awaitable did raise TypeError if the argument passed was not a callable.
msg263595 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-04-17 01:40
New changeset eef8f72ddb00 by Martin Panter in branch '2.7':
Issue #4806: Avoid masking TypeError when *-unpacking a generator
https://hg.python.org/cpython/rev/eef8f72ddb00
msg315358 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2018-04-16 14:11
I believe I encountered this issue today on Python 2.7.14 (https://ci.appveyor.com/project/jaraco/jaraco-windows/build/31/job/lenh5l4dcmj137b9). In this case, I have an iterable (in itertools.imap) that raises a TypeError when evaluated, not a generator.

The issue doesn't happen on Python 3, where 'map' is used instead of itertools.imap.

Does this patch need to be extended to support any iterable?
History
Date User Action Args
2022-04-11 14:56:43adminsetgithub: 49056
2018-04-16 14:11:55jaracosetnosy: + jaraco
messages: + msg315358
2016-04-17 04:00:20martin.pantersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-04-17 01:40:25python-devsetmessages: + msg263595
2016-02-15 09:42:11Michel Desmoulinsetmessages: + msg260304
2016-02-15 09:04:18martin.pantersetmessages: + msg260302
2016-02-15 08:34:35Michel Desmoulinsetnosy: + Michel Desmoulin
messages: + msg260300
2016-01-30 11:21:46martin.pantersetfiles: + check-generator-py2.patch

messages: + msg259251
stage: commit review -> patch review
2016-01-30 06:47:41python-devsetnosy: + python-dev
messages: + msg259240
2016-01-30 06:18:54martin.pantersetstage: patch review -> commit review
messages: + msg259239
versions: + Python 3.6, - Python 3.4
2016-01-22 16:06:32vstinnersetnosy: + yselivanov
2016-01-20 15:59:36Samuel BOVÉEsetnosy: + Samuel BOVÉE
messages: + msg258688
2015-02-24 04:29:14martin.pantersetfiles: + issue4806-star-TypeError.v2.patch

messages: + msg236475
2014-03-19 12:54:41r.david.murraysetnosy: + r.david.murray, benjamin.peterson

messages: + msg214091
versions: + Python 3.4, Python 3.5, - Python 3.2, Python 3.3
2014-03-19 05:32:26ned.deilylinkissue20959 superseder
2014-02-19 03:59:31pfctdayelisesetnosy: + pfctdayelise
2012-11-28 19:28:15DragonFireCKsetnosy: + DragonFireCK
2012-02-27 20:59:13brandjonsetnosy: + brandjon
2012-02-04 02:31:16terry.reedysetmessages: + msg152584
2012-02-04 02:28:38terry.reedysetnosy: + georg.brandl, pitrou, gpolo
messages: + msg152583
2012-02-04 02:27:29terry.reedylinkissue5218 superseder
2012-02-04 02:21:20terry.reedysetnosy: + ron_adam
2012-02-04 02:13:16terry.reedysetnosy: + ncoghlan

messages: + msg152580
versions: - Python 3.1
2012-02-04 02:12:56terry.reedylinkissue13904 superseder
2012-02-04 02:06:33terry.reedylinkissue11944 superseder
2012-01-12 04:51:33martin.pantersetmessages: + msg151110
2012-01-12 04:40:48martin.pantersetnosy: + martin.panter
2011-04-29 23:04:25terry.reedysetmessages: + msg134821
2011-03-07 07:21:24daniel.urbansetfiles: + issue4806.patch
nosy: terry.reedy, amaury.forgeotdarc, dangyogi, hagen, kcarnold, daniel.urban
messages: + msg130232
2011-03-06 12:35:11daniel.urbansetnosy: terry.reedy, amaury.forgeotdarc, dangyogi, hagen, kcarnold, daniel.urban
messages: + msg130168
versions: + Python 3.3
2011-03-06 09:47:11daniel.urbansetnosy: + daniel.urban
2010-08-05 19:13:55hagensetfiles: + combined.patch

messages: + msg113022
2010-08-05 16:34:09terry.reedysetmessages: + msg112998
2010-08-05 08:55:49hagensetmessages: + msg112960
2010-08-04 21:08:09terry.reedysetnosy: + terry.reedy

messages: + msg112896
versions: + Python 3.2, - Python 2.6, Python 2.5, Python 3.0
2010-05-03 18:03:26belopolskysetkeywords: + needs review
stage: patch review
2010-05-03 17:51:53dangyogisetnosy: + dangyogi

messages: + msg104861
versions: + Python 2.5
2009-12-29 16:41:47amaury.forgeotdarclinkissue7548 superseder
2009-06-03 11:46:39hagensetfiles: - TypeError.patch
2009-06-03 11:46:33hagensetfiles: + TypeError2.patch

messages: + msg88807
2009-06-03 11:10:15amaury.forgeotdarcsetmessages: + msg88803
2009-06-03 09:14:33hagensetfiles: + message_and_docs.patch
2009-06-03 09:13:41hagensetfiles: + TypeError.patch
keywords: + patch
messages: + msg88800

versions: + Python 3.1, Python 2.7
2009-06-02 22:43:01kcarnoldsetnosy: + kcarnold
messages: + msg88770
2009-01-03 13:44:56hagensetmessages: + msg78957
2009-01-02 14:26:26amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg78800
2009-01-02 12:46:02hagencreate