Can someone help me with this bug?

[Duncan Smith]

Hello,
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.

Duncan

 

[Peter Otten]

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:
.... 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
....
It looks like inplace modification, but isn't:
(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):
.... def __iand__(self, other):
.... self.value += other.value
.... return self
....(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

 

[Duncan Smith]

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:
... 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
...
It looks like inplace modification, but isn't:
(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):
... def __iand__(self, other):
... self.value += other.value
... return self
...(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

Yep. This also explains why the other unit tests involving __iand__ passed,
leaving me thinking it was OK. Cheers.

Duncan

 

[Dan Bishop]

Hello,
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 __iand__ method doesn't work the way that you think it does.
Remember, "self" is just another local variable name, so your function
works as if you had done:

def __iand__(self, other):
x = self.intersection(other)
return x

which (incorrectly) has the same effect as __and__.