Can someone help me with this bug?

[Peter Otten]

Duncan Smith wrote:

>          I'm probably missing something that should be obvious, but can
> anyone tell me what's wrong with the following?
> 
> temp.py
> ---------------------------------------------------------------------
> 
> class Item(object):
>     def __init__(self, id, data):
>         self.id = id
>         self.data = data
> 
> 
> class KeyedSet(dict):
>     def __init__(self, items=None):
>         if items is not None:
>             for item in items:
>                 self[item.id] = item
> 
>     def __iter__(self):
>         return self.itervalues()
> 
>     def __repr__(self):
>         return '%s(%r)' % (self.__class__.__name__, self.keys())
> 
>     def __contains__(self, item):
>         return self.has_key(item.id)
> 
>     def intersection(self, other):
>         res = self.__class__()
>         for item in self:
>             if item in other:
>                 res[item.id] = item
>         return res
> 
>     def __and__(self, other):
>         return self.intersection(other)
> 
>     def intersection_update(self, other):
>         self &= other
> 
>     def __iand__(self, other):
>         self = self.intersection(other)
>         return self
> --------------------------------------------------------------------
> 
>>>> from temp import Item, KeyedSet
>>>> a = Item(0, 'W')
>>>> b = Item(1, 'X')
>>>> c = Item(2, 'Y')
>>>> d = Item(3, 'Z')
>>>> aset = KeyedSet([a, b, c])
>>>> bset = KeyedSet([b, c, d])
>>>> aset &= bset
>>>> aset
> KeyedSet([1, 2])
>>>> aset = KeyedSet([a, b, c])
>>>> aset.intersection_update(bset)
>>>> aset
> KeyedSet([0, 1, 2])
>>>>
> 
> I can't figure out why 'aset' is unchanged?  Thanks.

Your problem stripped down to the bare bones: __iand__() creates a new
instance instead of modifying the current one. 

self = something 

doesn't copy something's data to self, it just rebinds self to something for
the rest of the method. 

A minimal example of what went wrong:

>>> class A:
...     def __init__(self, value):
...             self.value = value
...     def __iand__(self, other):
...             return A(self.value + other.value)
...     def __repr__(self):
...             return "value=%s" % self.value
...
>>> a = b = A(1)
>>> a &= A(2)

It looks like inplace modification, but isn't:

>>> a, b
(value=3, value=1)

a is rebound to the newly created instance, which tricks you into believing
it was modified. The backup reference b reveals the error.
Here's the correction (I'm lazy, so I reuse the unaltered parts of A):

>>> class B(A):
...     def __iand__(self, other):
...             self.value += other.value
...             return self
...
>>> a = b = B(1)
>>> a &= B(2)
>>> a, b
(value=3, value=3)

Once you understand the basic principle, it should be no problem to apply it
to your KeyedSet class. If in doubt, use the sets.Set implementation as a
template.

Peter