classification
Title: ItemsView.__contains__ does not mimic dict_items
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: clevy, martin.panter, rhettinger, serhiy.storchaka, stutzbach, xiang.zhang
Priority: normal Keywords: patch

Created on 2015-06-11 18:57 by clevy, last changed 2016-04-30 06:24 by xiang.zhang.

Files
File name Uploaded Description Edit
ItemsView_contains.patch clevy, 2015-06-11 19:20 review
Messages (9)
msg245182 - (view) Author: Caleb Levy (clevy) * Date: 2015-06-11 19:20
The current implementation ItemsView.__contains__ reads

class ItemsView(MappingView, Set):
    ...
    def __contains__(self, item):
        key, value = item
        try:
            v = self._mapping[key]
        except KeyError:
            return False
        else:
            return v == value
    ...

This poses several problems. First, any non-iterable or iterable not having exactly two elements will raise an error instead of returning false. 

Second, an ItemsView object is roughly the same as a set of tuple-pairs hashed by the first element. Thus, for example,

    ["a", 1] in d.items()

will return False for any dict d, yet in the current ItemsView implementation, this is True.

The patch changes behavior to immediately return false for non-tuple items and tuples not of length 2, avoiding unnecessary exceptions. It also adds tests to collections which fail under the old behavior and pass with the update.
msg245200 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-06-11 22:56
Added a couple suggestions for the test case on Reitveld.
msg245207 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-06-12 04:58
Additional check hits performance. First issue can be resolved by changing code to

    try:
        key, value = item
    except TypeError:
        return False

Second issue can be resolved by comparing not v with value, but (key, v) with item. However I'm not sure that fixing it is worth such complication of the code.
msg245211 - (view) Author: Caleb Levy (clevy) * Date: 2015-06-12 05:48
@serhiy.storchaka: I don't think that will work.

First of all,

    x, y = item

will raise a ValueError if fed an iterable whose length is not exactly 2, so you would have to check for that. Moreover, if item is something like a dict, for example, then:

    {"a": 1, "b": 2} in DictLikeMapping(a="b")

could return True, which I don't think would be expected behavior.

I'm not terribly fond of the instance check myself, but in this case I can't see any other way to do it: the built in dict_items necessarily consists of *tuples* of key-value pairs.
msg245212 - (view) Author: Caleb Levy (clevy) * Date: 2015-06-12 05:52
Sorry; that should be DictLikeMapping(a="b").items(), where DictLikeMapping is defined in the patch unit tests.
msg245416 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-06-16 04:23
The trouble with Serhiy’s suggestion is that it would still try to iterate the argument:

>>> i = iter(lambda: print("ITERATION"), "infinity")
>>> i in dict()  # No iteration
False
>>> i in ItemsView(dict())
ITERATION
ITERATION
ITERATION
False
msg256741 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-19 21:41
Yet one corner case:

>>> (1, math.nan) in {1: math.nan}.items()
True
>>> (1, math.nan) in ItemsView({1: math.nan})
False

This can be resolved if compare not v with value, but a tuple (key, v) with item.
msg264487 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-04-29 10:45
Caleb's resolution looks good, just like the C version does, at least seems correct.

Serhiy, the corner case is interesting. math.nan == math.nan should return false so I think (1, math.nan) in ItemsView({1: math.nan} is a right behaviour. But the C version, which calls PyObject_RichCompareBool and then do a pointer check first seems having some problem.
msg264543 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-04-30 06:24
After reading issue4296, I agree with Serhiy's point on the second issue. Right now, (1, math.nan) in ItemsView({1: math.nan}) returns false which seems not acceptable.
History
Date User Action Args
2016-04-30 06:24:23xiang.zhangsetmessages: + msg264543
2016-04-29 10:45:20xiang.zhangsetmessages: + msg264487
2016-04-28 09:54:15xiang.zhangsetnosy: + xiang.zhang
2016-04-26 08:14:22rhettingersetassignee: rhettinger ->
2015-12-19 21:41:12serhiy.storchakasetmessages: + msg256741
2015-06-16 04:23:41martin.pantersetmessages: + msg245416
2015-06-12 05:52:41clevysetmessages: + msg245212
2015-06-12 05:48:17clevysetmessages: + msg245211
2015-06-12 04:58:40serhiy.storchakasetnosy: + serhiy.storchaka

messages: + msg245207
versions: - Python 3.2, Python 3.3
2015-06-12 01:53:43rhettingersetassignee: rhettinger
2015-06-11 22:56:18martin.pantersetnosy: + martin.panter

messages: + msg245200
stage: patch review
2015-06-11 19:20:51clevysetfiles: + ItemsView_contains.patch
keywords: + patch
messages: + msg245182
2015-06-11 18:57:53clevycreate