[Python-bugs-list] [ python-Bugs-628246 ] Set constructor fails with NameError
noreply@sourceforge.net
noreply@sourceforge.net
Thu, 07 Nov 2002 17:06:42 -0800
Bugs item #628246, was opened at 2002-10-24 16:52
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=628246&group_id=5470
Category: Python Library
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Jeremy Hylton (jhylton)
Assigned to: Guido van Rossum (gvanrossum)
Summary: Set constructor fails with NameError
Initial Comment:
Here is a toy demo of the problem:
def baditer():
raise TypeError
yield 1
import sets
sets.Set(baditer())
Traceback (most recent call last):
File "/tmp/baditer.py", line 8, in ?
sets.Set(baditer())
File "/usr/local/lib/python2.3/sets.py", line 367, in
__init__
self._update(iterable)
File "/usr/local/lib/python2.3/sets.py", line 330, in
_update
transform = getattr(element, "_as_immutable", None)
UnboundLocalError: local variable 'element' referenced
before assignment
The _update() method has a try/except for TypeError
that is trying to catch failures to place set elements
in the dictionary. The try/except also wraps a for
loop over an iterator. So if something goes wrong and
the iterator raises TypeError, the
failed-to-insert-element-in-dict code is run.
The obvious solution, it seems, is to have the
try/except inside the for loop so that it only catches
errors in dict insertion. Tim points out that this
would be slow.
The example above is a toy, but I ran into this bug in
a real application where debugging was made harded
because the Set code was catching an exception when it
shouldn't.
----------------------------------------------------------------------
>Comment By: Guido van Rossum (gvanrossum)
Date: 2002-11-07 20:06
Message:
Logged In: YES
user_id=6380
Yikes! You meant type(iterable), not type(iter).
I expect this will get complaints from people who write
careful iterators.
But it seems the best compromise for now, so check it in
(with above fix). I doubt this is the last word though. :-(
It needs a unittest that tries this with each of the
mentioned types as well as with something that's not one of
them.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-11-07 18:07
Message:
Logged In: YES
user_id=80475
Revised patch that avoids performance issues in cases
where the iterator won't raise errors during iteration.
For the other cases, list(iter) is still faster than inserting a
try/except inside the loop.
Please pronounce one way or the other and then re-assign
to me. I'll move the try/except back inside the loop if
that's what you want.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-11-02 19:02
Message:
Logged In: YES
user_id=6380
Hmm... It's the performance waste of always making an
unnecessary copy (the argument will often already be a list,
or a tuple) vs. the performance waste of doing try/except
each iteration.
Maybe it should all be replaced by this?
for element in iterable:
transform = getattr(element, "_as_immutable", None)
if transform is not None:
element = transform()
data[element] = value
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-10-31 23:09
Message:
Logged In: YES
user_id=80475
Yes. The intended behavior is to have list(iterable) run the
iterable start to finish so that any exceptions raised will
not be trapped by the Sets code during dictionary
construction.
Feel free to disregard the idea. I thought it had some merit
because it is simple, fast and fixes the problem. Still, it
looks weird and I won't lose any sleep if you toss the idea.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-10-31 21:03
Message:
Logged In: YES
user_id=6380
That patch makes no sense -- it first materializes the
iterator as a list, then iterates over that. Was that what
you intended?
I'd rather give up on the speediness and put the try/except
where it belongs.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-10-25 23:39
Message:
Logged In: YES
user_id=80475
A simple solution is to run through the iterator once before
starting the update. A one line patch is attached.
The only thing I don't like about it is that the code is
starting to look hare-brained.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=628246&group_id=5470