Add 'in' operator support to IPTCInfo and IPTCData

[rufoa]

So we can do

info = IPTCInfo('foo.jpg')
if 'caption/abstract' in info:

etc.

Many thanks

[rufoa]

Sorry - I should have been clearer in my original post.

The IPTCData class actually already has in support, which it inherits from dict. The class then overrides the __getitem__/__setitem__ methods, so that the dict elements can also be accessed using friendly key names like 'caption/abstract' as well as its real dict key 120.

However it does not also override the __contains__ method, responsible for in tests, in a corresponding way. This means that e.g. if 105 in info: works correctly, but if 'headline' in info: does not. But the latter doesn't simply return False as one might expect - it actually hangs in an infinite loop. :confused:

Thankfully the class already contains the solution for dealing with string keys, in the implementation of __getitem__: you use _key_as_int_ to convert the (possibly string) key to its corresponding number first. So I overrode the __contains__ method, and have it call _key_as_int_ before passing the now-definitely-numeric key to the underlying dict's __contains__ method, as before.

This fixes the strange infinite loop bug, and makes 'example' in info work as expected. :+1:

Are you sure this is feature complete before I do this?

Yes, I can't think of any further changes that would be needed once this is merged.

Does this save CPU / time doing the in operator vs the getitem look up an searching through the keys? Do we have benchmarks on that?

I'm not sure what you mean here, sorry. My changes to the in lookup do not affect its performance characteristics, because the lookup is still handled by the underlying dict's in operator, as before. It's neither faster nor slower.

I am curious why the original developer did not include this in the original IPTCInfo package,

I was surprised by this too. I can only assume he never actually tested it (NB I have added appropriate new test cases to catch the bug), or he would have realised it causes python to hang!

basically are there any negatives to adding this operator?

None I can think of. It fixes a bug with the existing in operator, and allows people to use it with string keys if they wish, like they can with the getter/setter. Sounds like a win/win to me.

Hope this explains everything! Thanks for reviewing the changes

[james-see]

@rufoa thank you for that very clear explanation. I am trying really hard to not make this bloated or anything extraneous, now I see very clearly that this fixes a nasty bug. This is the exact type of pull request I love seeing, and really appreciate the contribution. I am going to merge, but it may take a day or two for me to get to pushing the new version to pypi.

[rufoa]

Great! No rush getting it pushed to pypi. Thanks