| msg13979 - (view) |
Author: Walter Dörwald (doerwalter) *  |
Date: 2003-01-10 14:41 |
| In the following test script ----- class Test: def __iter__(self): raise IOError reduce(lambda x,y: x+y, Test()) ----- the real IOError exception is masked, i.e. the traceback is ----- Traceback (most recent call last): File "test.py", line 5, in ? reduce(lambda x,y: x+y, Test()) TypeError: reduce() arg 2 must support iteration ----- but IMHO should be ----- Traceback (most recent call last): File "test.py", line 3, in ? raise IOError IOError ----- This can be fixed by removing the PyErr_SetString(PyExc_TypeError, "reduce() arg 2 must support iteration") call in bltinmodule.c/buildtin_reduce(). |
|
|
| msg13980 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2003-01-11 17:08 |
| Logged In: YES user_id=366566 the __iter__ method is supposed to return an object that defines a 'next' method. The returned object is the one used for iteration, not the original. So I believe the error message is correct - Test does not support iteration. If you change the code to: >>> class test: ... def __iter__(self): ... return self ... def next(self): ... raise IOError ... >>> reduce(operator.add, test()) You get the expected result... Traceback (most recent call last): File "", line 1, in ? File "", line 5, in next IOError |
|
|
| msg13981 - (view) |
Author: Walter Dörwald (doerwalter) * |
Date: 2003-01-13 13:18 |
| Logged In: YES user_id=89016 The point is that Python/bltinmodule.c/builtin_reduce() masks the error returned from PyObject_GetIter(). Errors from PyIter_Next() are not masked. What about the following example: class LazyFile: def __init__(self, name, mode="r"): self.name = name self.mode = mode def __iter__(self): return open(self.name, self.mode) import operator f = LazyFile("does not exist") s = reduce(operator.add, f) LazyFile *does* support iteration, but the underlying problem of the non existing file is masked. Removing the call PyErr_SetString(PyExc_TypeError, "reduce() arg 2 must support iteration"); in builtin_reduce(), will produce the original exception "IOError: [Errno 2] No such file or directory: 'does not exist'" and when the second argument is not iteratable, the original exception is just as good: >>> reduce(lambda x,y: x+y, 42) Traceback (most recent call last): File "", line 1, in ? TypeError: iteration over non-sequence |
|
|
| msg13982 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2003-01-25 04:00 |
| Logged In: YES user_id=80475 There's a lot of this going around. map() and zip() have the same issue. I recommend fixing them all. |
|
|
| msg13983 - (view) |
Author: Walter Dörwald (doerwalter) * |
Date: 2003-01-28 20:38 |
| Logged In: YES user_id=89016 Attached is a patch that fixes reduce(), map() and zip(). Unfortunately this loses the information about which argument triggered the exception (in map() and zip()) |
|
|
| msg13984 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2003-01-28 23:59 |
| Logged In: YES user_id=80475 One way to mitigate the information loss is to mimic the style in zip() which only traps TypeErrors but passes through things like the IOError in your original example. |
|
|
| msg13985 - (view) |
Author: Walter Dörwald (doerwalter) * |
Date: 2003-01-30 11:29 |
| Logged In: YES user_id=89016 Trapping only TypeError won't help: class LazyFile: def __init__(self, name, mode="r"): self.name = name self.mode = mode def __iter__(self): return open(self.name, self.mode) import operator f = LazyFile(42) s = reduce(operator.add, f) Here the open call will raise a TypeError, that is totally unrelated to the iterator protocol. The cleanest solution would really be exception chaining. |
|
|
| msg13986 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2006-12-21 15:35 |
| Preserving the argument number seems difficult without exception chaining; perhaps a middle group would be to only replace TypeError exceptions. Walter's comment at 2003-01-30 06:29 shows this can still be fooled, but a fix for 90% of the cases is still better than a fix for 0% of them. The patch looks OK, but I think it should be reworked to take the middle-ground approach, replacing only TypeErrors. |
|
|
| msg81849 - (view) |
Author: Daniel Diniz (ajaksu2) *  |
Date: 2009-02-13 02:31 |
| Confirmed in trunk, rev69550. |
|
|
| msg104685 - (view) |
Author: Alexander Belopolsky (Alexander.Belopolsky) |
Date: 2010-05-01 01:25 |
| I am attaching a patch with unit tests that implements the "middle-ground approach" making map and reduce behave the way zip is now. I my view this slightly preferable to the "all the way" approach of letting all exceptions to propagate up. My reasoning is that out of the three functions, zip, reduce and map, zip is probably the most common and changing its behavior is more likely to affect somebody than a change to the other two. |
|
|
| msg114066 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2010-08-16 19:10 |
| Committed to py3k in r84098. Accepting this change for py3k was an easy decision to make because zip and map already behave this way in 3.x. I am inclined to reject this for 2.7, however. While I agree that this is a bug, fixing it has a potential of breaking users' code. I also note that for zip and map, 2.7 users can switch to izip and imap which don't have this problem. Arguably, switching to izip and imap in new code is a good idea regardless of this issue. While there is no similar work-around for reduce, I don't think this bug is important enough to introduce backward incompatible change in the stable series. |
|
|
| msg116339 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2010-09-13 18:48 |
| Since noone have spoken in favor of 2.7 backport, I am closing this issue as committed to py3k. |
|
|