This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: dict_values isn't considered a Collection nor a Container
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: rhettinger, yahya-abou-imran
Priority: low Keywords: patch

Created on 2017-12-31 15:36 by yahya-abou-imran, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5152 merged rhettinger, 2018-01-11 08:37
Messages (4)
msg309289 - (view) Author: Yahya Abou Imran (yahya-abou-imran) * Date: 2017-12-31 15:36
a `dict_values` instance behaves like a Collection (a Sized Iterable Container):

>>> values = {1: 'one', 2: 'two'}.values()
>>> 'two' in values
True
>>> 'three' in values
False
>>> for value in values: print(value)
one
two
>>> len(values)
2

But...

>>> isinstance(values, abc.Collection)
False
>>> isinstance(values, abc.Container)
False

The reason is the lack of __contains__():

>>> '__contains__' in dir(values)
False

The 'in' operator works above because it uses __iter__() to check the presence of the value.

I think it's inconsistent with the fact that dict_values is in the registry of ValuesView witch passes the test:

>>> issubclass(abc.ValuesView, abc.Collection)
True

A class passed in the registry of ValuesView should guarantee this behaviour (witch is the case here by the way, but informally).


I can think about several solutions for this:

1. add a __contains__() to the dict_values class;
2. if an ABC is considered a subclass of another ABC, all the classes in  the registry of the first (and recursively all of their subclasses) should be too;
3. pass dict_values in the registry of Container or Collection;
4. make ValuesView inherit from Container or Collection.


IMHO:

1 is out.
2 makes sense, but may be difficult to implement since it implies that a class has to know all the registries it's been passed in.
3 and 4 are by far the easiest ways to do it.


I think the inheritance from Collection is the best solution, because KeysView and ItemsView are already Collections by inheriting from Set witch inherits from Collection. The only fondamental difference between the three views (in terms of protocol and API), it's that ValuesView is not a Set.
msg309795 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-11 08:35
It would be okay to let ValuesView inherit from Collection.

Am marking this a 3.7 only because it isn't important to anyone's actual code and there is no known use case.  Technically, it isn't even a bug.  The __contains__ method is supported implicitly (as it is for many classes that just define __iter__), so there is no requirement that this be recognized as a Collection.  Also, the existing behavior was explicitly tested (see line 848 in Lib/test/test_collections).

I'm going forward with this because it does offer a sense of neatness (in comparison to KeysView and ItemsView) and it might avoid a pedantic StackOverflow question somewhere down the line.
msg309813 - (view) Author: Yahya Abou Imran (yahya-abou-imran) * Date: 2018-01-11 15:04
Perfect for me.
I was about to submit about the same patch as yours after your acceptation; but it's better that way, it's your module!
Thanks Raymond.
msg309837 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-12 05:53
New changeset 02556fbade5e1e864dd09d5768a8dbbf5b3a0dac by Raymond Hettinger in branch 'master':
bpo-32467: Let collections.abc.ValuesView inherit from Collection (#5152)
https://github.com/python/cpython/commit/02556fbade5e1e864dd09d5768a8dbbf5b3a0dac
History
Date User Action Args
2022-04-11 14:58:56adminsetgithub: 76648
2018-01-12 05:54:15rhettingersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-01-12 05:53:52rhettingersetmessages: + msg309837
2018-01-11 15:04:14yahya-abou-imransetmessages: + msg309813
2018-01-11 08:37:01rhettingersetkeywords: + patch
stage: patch review
pull_requests: + pull_request5007
2018-01-11 08:35:05rhettingersetpriority: normal -> low

assignee: rhettinger
components: + Library (Lib)
versions: - Python 3.6
nosy: + rhettinger

messages: + msg309795
2017-12-31 15:36:24yahya-abou-imrancreate