Skip to content

contextvars.Context doesn't inherit from Mapping #12873

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

Closed
wants to merge 1 commit into from

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Oct 22, 2024

It's not a subclass, it's not registered to Mapping, and Mapping isn't protocol-like, so at runtime:

>>> issubclass(Context, Mapping)
False

It does have all the necessary methods to be Mapping, it just isn't.

Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood
Copy link
Member

That's kind of interesting that we don't do the collections.abc.Mapping.register(Context) thing at runtime; feels like an omission over at CPython

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, but the primer hits are encouraging.

@tungol
Copy link
Contributor Author

tungol commented Oct 22, 2024

Yeah, I was wondering if maybe the right thing to do here was add the registration to cpython?

I haven't looked at them in detail yet, but two others in a somewhat similar boat are multiprocessing.managers.DictProxy and multiprocessing.managers.BaseListProxy. They're MutableMapping and MutableSequence in typeshed, respectively, but neither is actually registered to the ABC at runtime. They're on my "investigate" list coming up.

@AlexWaygood
Copy link
Member

Yeah, I was wondering if maybe the right thing to do here was add the registration to cpython?

Very possibly. In theory of course, we could do this change and the CPython change, though it might lead to some annoying branching in the stubs if the CPython change is only accepted for Python 3.14+

@tungol
Copy link
Contributor Author

tungol commented Nov 6, 2024

This is now fixed in cpython for 3.12 and up. It probably doesn't make sense to branch for the sake of 3.8 to 3.11.

@tungol tungol closed this Nov 6, 2024
@tungol tungol deleted the contextvars branch November 7, 2024 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants