matplotlib-devel

[matplotlib-devel] cbook.py, maxdict class key checking on __setitem__

[Ryanitus <rya...@ho...>]

I may have found a bug in the __setitem__ method of the maxdict class.

Since a dictionary is a mapping class, if an item is set that already
exists, it overwrites the previous.  However, you are still appending that
item to _killkeys regardless.

In the case where 2 items with the same key were added and later removed due
to size constraints, the line in bold would throw an exception on the second
call.

By checking for the key existance, you handle that situation gracefully. 
The item can still be removed from _killkeys, since it's the next to go
anyway.

Ryan

Original code (circa line 776):

        if len(self)>=self.maxsize:
            del self[self._killkeys[0]]
            del self._killkeys[0]
        dict.__setitem__(self, k, v)
        self._killkeys.append(k)

Possible solution:

        if len(self)>=self.maxsize:
            if self.has_key(self._killkeys[0]):
                del self[self._killkeys[0]]
            del self._killkeys[0]
        dict.__setitem__(self, k, v)
        self._killkeys.append(k)



-- 
View this message in context: http://www.nabble.com/cbook.py%2C-maxdict-class-key-checking-on-__setitem__-tp25090453p25090453.html
Sent from the matplotlib - devel mailing list archive at Nabble.com.

Re: [matplotlib-devel] cbook.py, maxdict class key checking on __setitem__

[Eric F. <ef...@ha...>]

Ryanitus wrote:
> I may have found a bug in the __setitem__ method of the maxdict class.
> 

Fixed--thanks for pointing it out.

Eric

> Since a dictionary is a mapping class, if an item is set that already
> exists, it overwrites the previous.  However, you are still appending that
> item to _killkeys regardless.
> 
> In the case where 2 items with the same key were added and later removed due
> to size constraints, the line in bold would throw an exception on the second
> call.
> 
> By checking for the key existance, you handle that situation gracefully. 
> The item can still be removed from _killkeys, since it's the next to go
> anyway.
> 
> Ryan
> 
> Original code (circa line 776):
> 
>         if len(self)>=self.maxsize:
>             del self[self._killkeys[0]]
>             del self._killkeys[0]
>         dict.__setitem__(self, k, v)
>         self._killkeys.append(k)
> 
> Possible solution:
> 
>         if len(self)>=self.maxsize:
>             if self.has_key(self._killkeys[0]):
>                 del self[self._killkeys[0]]
>             del self._killkeys[0]
>         dict.__setitem__(self, k, v)
>         self._killkeys.append(k)
> 
> 
>