Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dict_values isn't considered a Collection nor a Container #76648

Closed
yahya-abou-imran mannequin opened this issue Dec 31, 2017 · 4 comments
Closed

dict_values isn't considered a Collection nor a Container #76648

yahya-abou-imran mannequin opened this issue Dec 31, 2017 · 4 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@yahya-abou-imran
Copy link
Mannequin

yahya-abou-imran mannequin commented Dec 31, 2017

BPO 32467
Nosy @rhettinger, @yahya-abou-imran
PRs
  • bpo-32467: Let collections.abc.ValuesView inherit from Collection #5152
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/rhettinger'
    closed_at = <Date 2018-01-12.05:54:15.840>
    created_at = <Date 2017-12-31.15:36:24.073>
    labels = ['3.7', 'type-bug', 'library']
    title = "dict_values isn't considered a Collection nor a Container"
    updated_at = <Date 2018-01-12.05:54:15.840>
    user = 'https://github.com/yahya-abou-imran'

    bugs.python.org fields:

    activity = <Date 2018-01-12.05:54:15.840>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2018-01-12.05:54:15.840>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2017-12-31.15:36:24.073>
    creator = 'yahya-abou-imran'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32467
    keywords = ['patch']
    message_count = 4.0
    messages = ['309289', '309795', '309813', '309837']
    nosy_count = 2.0
    nosy_names = ['rhettinger', 'yahya-abou-imran']
    pr_nums = ['5152']
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue32467'
    versions = ['Python 3.7']

    @yahya-abou-imran
    Copy link
    Mannequin Author

    yahya-abou-imran mannequin commented Dec 31, 2017

    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.

    @yahya-abou-imran yahya-abou-imran mannequin added 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Dec 31, 2017
    @rhettinger
    Copy link
    Contributor

    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.

    @rhettinger rhettinger added the stdlib Python modules in the Lib dir label Jan 11, 2018
    @rhettinger rhettinger self-assigned this Jan 11, 2018
    @yahya-abou-imran
    Copy link
    Mannequin Author

    yahya-abou-imran mannequin commented Jan 11, 2018

    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.

    @rhettinger
    Copy link
    Contributor

    New changeset 02556fb by Raymond Hettinger in branch 'master':
    bpo-32467: Let collections.abc.ValuesView inherit from Collection (bpo-5152)
    02556fb

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant