Skip to content

register contextvars.Context to collections.abc.Mapping #126451

Closed
Closed
register contextvars.Context to collections.abc.Mapping#126451
Assignees
AlexWaygood
Labels
stdlibStandard Library Python modules in the Lib/ directorytopic-typingtype-featureA feature request or enhancement

Description

@tungol
tungol
opened on Nov 5, 2024

Feature or enhancement

Proposal:

contextvars.Context is documented as implementing the collections.abc.Mapping interface. I believe we should register it to the abc, so an isinstance check can work.

This is similar to #126417 .

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

A small amount of prior discussion occurred in an issue for typeshed: python/typeshed#12873

Linked PRs

Activity

  1. tungol
    added
    type-featureA feature request or enhancement
    on Nov 5, 2024

  2. JelleZijlstra commented on Nov 5, 2024

    @JelleZijlstra
    JelleZijlstra
    on Nov 5, 2024
    Member

    Could you list out all the methods on Mapping to confirm it really conforms with the interface? I'd like to prevent another case similar to #125420.

  3. tungol commented on Nov 5, 2024

    @tungol
    tungol
    on Nov 5, 2024
    ContributorAuthor

    Sure.

    Mapping methods are: __getitem__, __iter__, __len__, __contains__, keys, items, values, get, __eq__, and __ne__. All are present on Context:

    >>> from contextvars import Context
>>> Context.__getitem__
<slot wrapper '__getitem__' of '_contextvars.Context' objects>
>>> Context.__iter__
<slot wrapper '__iter__' of '_contextvars.Context' objects>
>>> Context.__len__
<slot wrapper '__len__' of '_contextvars.Context' objects>
>>> Context.__contains__
<slot wrapper '__contains__' of '_contextvars.Context' objects>
>>> Context.keys
<method 'keys' of '_contextvars.Context' objects>
>>> Context.items
<method 'items' of '_contextvars.Context' objects>
>>> Context.values
<method 'values' of '_contextvars.Context' objects>
>>> Context.get
<method 'get' of '_contextvars.Context' objects>
>>> Context.__eq__
<slot wrapper '__eq__' of '_contextvars.Context' objects>
>>> Context.__ne__
<slot wrapper '__ne__' of '_contextvars.Context' objects>
  4. picnixz
    added
    topic-typing
    stdlibStandard Library Python modules in the Lib/ directory
    on Nov 6, 2024
  5. sobolevn
    added a commit that references this issue on Nov 6, 2024

    gh-126451: Register contextvars.Context to collections.abc.Mapping (#…

    5dc36dc
  6. miss-islington
    added 2 commits that reference this issue on Nov 6, 2024

    pythongh-126451: Register contextvars.Context to collections.abc.Mapp…

    a307416

    pythongh-126451: Register contextvars.Context to collections.abc.Mapp…

    eec74ce
  7. sobolevn
    added 2 commits that reference this issue on Nov 6, 2024

    [3.12] gh-126451: Register contextvars.Context to collections.abc.Map…

    d71da0f

    [3.13] gh-126451: Register contextvars.Context to collections.abc.Map…

    59316a6
  8. tungol
    closed this as completedon Nov 6, 2024

  9. hauntsaninja commented on Nov 6, 2024

    @hauntsaninja
    hauntsaninja
    on Nov 6, 2024
    Contributor

    I was surprised that we backported this

  10. sobolevn commented on Nov 7, 2024

    @sobolevn
    sobolevn
    on Nov 7, 2024
    Member

    I was too :)
    But, we backported #126419
    So, I decided to be consistent. If you think that these backports should be reverted - then I have no objections :)

  11. AlexWaygood commented on Nov 7, 2024

    @AlexWaygood
    AlexWaygood
    on Nov 7, 2024
    Member

    The decision to backport is on me -- I applied the backport labels to both PRs. But I also don't feel strongly, and am fine with the backports being reverted if you disagree with them!

    FWIW, my thinking was:

    • I think this class was always meant to implement a mapping API, and I think it was just an oversight (a micro-bug, IMO) that it was never explicitly registered to the ABC
    • I couldn't see any risk of breakage from doing so; it seemed very low-risk

    But as I said in #126419 (comment), it feels very borderline to me, so I'm very happy to revert the backports if others disagree!

  12. JelleZijlstra commented on Nov 7, 2024

    @JelleZijlstra
    JelleZijlstra
    on Nov 7, 2024
    Member

    The risky aspect to me is that isinstance(..., Mapping) will start behaving differently in a patch release. That may have unexpected effects for some users.

  13. tungol commented on Nov 7, 2024

    @tungol
    tungol
    on Nov 7, 2024
    ContributorAuthor

    As the person who kicked this off: I have no opinion about the backports.

  14. AlexWaygood commented on Nov 11, 2024

    @AlexWaygood
    AlexWaygood
    on Nov 11, 2024
    Member

    The failing tests in #126674 and #126675 seem to validate folks' concerns about the backports of these PRs. I'll open a revert PR for the backports tomorrow.

  15. AlexWaygood
    reopened this on Nov 11, 2024
  16. AlexWaygood
    self-assigned this
    on Nov 11, 2024

  17. AlexWaygood commented on Nov 11, 2024

    @AlexWaygood
    AlexWaygood
    on Nov 11, 2024
    Member

    Thanks everybody for voicing your concerns! I appreciate it.

  18. AlexWaygood
    added 2 commits that reference this issue on Nov 12, 2024

    Revert "[3.13] pythongh-126451: Register contextvars.Context to colle…

    9705e20

    Revert "[3.12] pythongh-126451: Register contextvars.Context to colle…

    49d69f2

  19. AlexWaygood commented on Nov 12, 2024

    @AlexWaygood
    AlexWaygood
    on Nov 12, 2024
    Member

    Backports are being reverted in:

  20. AlexWaygood
    added 2 commits that reference this issue on Nov 12, 2024

    [3.12] gh-126451: Revert backports of ABC registrations for contextva…

    86efa2f

    [3.13] gh-126451: Revert backports of ABC registrations for `contextv…

    4f1440f
  21. AlexWaygood
    closed this as completedon Nov 12, 2024
  22. picnixz
    added a commit that references this issue on Dec 8, 2024

    pythongh-126451: Register contextvars.Context to collections.abc.Mapp…

    ba95f15
  23. ebonnal
    added a commit that references this issue on Jan 12, 2025

    pythongh-126451: Register contextvars.Context to collections.abc.Mapp…

    41a38f5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

stdlibStandard Library Python modules in the Lib/ directorytopic-typingtype-featureA feature request or enhancement

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions