Merge lp://qastaging/~antasi/pyexiv2/mutablemapping into lp://qastaging/pyexiv2/0.2.x

Thanks for the merge proposal!

I noticed the first revision (326) fixes a separate issue which wasn’t detected by the unit tests. Well spotted! I’m going to tweak this revision a bit and merge it first to keep things atomic, then I’ll review the actual implementation of the MutableMapping interface.

About the bug you fixed:
 - the unit tests didn’t catch this problem, I’ll update them in order to avoid regressions in the future
 - list.remove(x) may raise a ValueError, so the try… except… blocks should be adapted

As for the rest of the review, I’m running out of spare time this week and I’ll be away on holidays next week, but be assured that I’ll review it thoroughly when I get back. Thanks again for the valuable contribution!

I pushed revision 326 to lp:pyexiv2 that fixes the keys not being deleted from the cache when a tag is deleted. Would you mind reverting revision 326 in your branch and push? This will ensure the merge proposal and the diff to review are up-to-date.

A quick go at a functional review while I’m at it:

If I understand correctly the documentation of the collections module (http://docs.python.org/library/collections.html#abcs-abstract-base-classes), a class implementing MutableMapping should implement the following methods:

 - __len__ (from Sized)
 - __iter__ (from Iterable)
 - __contains__ (from Container)
 - __getitem__ (from Mapping)
 - __setitem__
 - __delitem__

Your implementation misses __contains__.

> A quick go at a functional review while I’m at it:
> If I understand correctly the documentation of the collections module
> (http://docs.python.org/library/collections.html#abcs-abstract-base-classes),
> ...
> Your implementation misses __contains__.

The __contains__ method comes as a mixin method from Mapping class that MutableMapping inherits, so it is not necessary to implement it.
>>> import pyexiv2
>>> i = pyexiv2.ImageMetadata('/var/tmp/20100703T131613-5fe0998c.jpg')
>>> i.read()
>>> i.__contains__
<bound method ImageMetadata.__contains__ of <pyexiv2.metadata.ImageMetadata object at 0xb7395a6c>>
>>>

It is not stated in the documentation, but I think it uses the implemented __iter__ method to go through the keys and check if the searched item is there.

Right, I overlooked that in the documentation.

Sorry for the very late review.
It’s all good, thanks a lot for your work.
I merged it into trunk at revision 327.

I will try to update the documentation in a near future to reflect the cool things that implementing the MutableMapping interface offers.