KEMBAR78
contextvars.Context doesn't inherit from Mapping by tungol · Pull Request #12873 · python/typeshed · GitHub
Skip to content

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.

@github-actions
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