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

weakref.WeakSet not instanceof collections.abc.Set #82942

Closed
Septatrix mannequin opened this issue Nov 10, 2019 · 5 comments
Closed

weakref.WeakSet not instanceof collections.abc.Set #82942

Septatrix mannequin opened this issue Nov 10, 2019 · 5 comments
Assignees
Labels
3.9 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@Septatrix
Copy link
Mannequin

Septatrix mannequin commented Nov 10, 2019

BPO 38761
Nosy @gvanrossum, @rhettinger, @septatrix
PRs
  • WIP bpo-38761: Register WeakSet as a MutableSet #17104
  • 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 2019-11-11.04:13:30.075>
    created_at = <Date 2019-11-10.11:39:15.194>
    labels = ['type-feature', 'library', '3.9']
    title = 'weakref.WeakSet not instanceof collections.abc.Set'
    updated_at = <Date 2019-11-11.04:13:30.073>
    user = 'https://github.com/Septatrix'

    bugs.python.org fields:

    activity = <Date 2019-11-11.04:13:30.073>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2019-11-11.04:13:30.075>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2019-11-10.11:39:15.194>
    creator = 'Nils Kattenbeck'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38761
    keywords = ['patch']
    message_count = 5.0
    messages = ['356326', '356337', '356343', '356349', '356350']
    nosy_count = 3.0
    nosy_names = ['gvanrossum', 'rhettinger', 'Nils Kattenbeck']
    pr_nums = ['17104']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue38761'
    versions = ['Python 3.9']

    @Septatrix
    Copy link
    Mannequin Author

    Septatrix mannequin commented Nov 10, 2019

    Instances of weakref.WeakSet are not instances of Set and therefore not of MutableSet but they are instances of Collection.
    They however implement all required methods for a MutableSet and Weak(Key|Value)Dictionary are correctly identified.
    Is this just an oversight or am I missing something?

    from weakref import WeakKeyDictionary, WeakValueDictionary, WeakSet
    from collections.abc import MutableMapping, Collection, Set, MutableSet
    
    wkdict = WeakKeyDictionary()
    wvdict = WeakValueDictionary()
    ws = WeakSet()

    assert isinstance(wkdict, MutableMapping)
    assert isinstance(wvdict, MutableMapping)
    assert isinstance(ws, Collection)
    assert not isinstance(ws, Set)
    assert not isinstance(ws, MutableSet)

    @Septatrix Septatrix mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir labels Nov 10, 2019
    @rhettinger
    Copy link
    Contributor

    We could register the WeakSet in _collections_abc, right after MutableSet is defined. That module only registered builtins so that it can avoid imports; however, since we know that WeakSet is already loaded, it is reasonable to add an import for registration purposes.

    This was likely omitted because WeakSet is used in the implementation of the ABC.

    Marking this as a proposed enhancement rather than as a bug because the collections ABCs are under no obligation to register anything more than the builtins.

    @rhettinger rhettinger added 3.9 only security fixes and removed 3.7 (EOL) end of life labels Nov 10, 2019
    @rhettinger rhettinger self-assigned this Nov 10, 2019
    @rhettinger rhettinger added the type-feature A feature request or enhancement label Nov 10, 2019
    @gvanrossum
    Copy link
    Member

    As I wrote between the lines in the PR, this would be a bug in the weakref module, not a bug in collections.abc.

    @rhettinger
    Copy link
    Contributor

    New changeset 84ac437 by Raymond Hettinger in branch 'master':
    bpo-38761: Register WeakSet as a MutableSet (GH-17104)
    84ac437

    @rhettinger
    Copy link
    Contributor

    Nils, thanks for the report.

    Guido, thanks for the clarification.

    @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.9 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants