[Python-bugs-list] [ python-Bugs-628246 ] Set constructor fails with NameError

noreply@sourceforge.net noreply@sourceforge.net
Sat, 02 Nov 2002 16:02:51 -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-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