FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

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

contextvars.Context doesn't inherit from Mapping - #12873

Closed
tungol wants to merge 1 commit into
python:mainfrom
tungol:contextvars
Closed

contextvars.Context doesn't inherit from Mapping#12873
tungol wants to merge 1 commit into
python:mainfrom
tungol:contextvars

Conversation

tungol commented Oct 22, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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

Copy link
Copy Markdown
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

srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Interesting, but the primer hits are encouraging.

tungol commented Oct 22, 2024

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
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 commented Nov 6, 2024

Copy link
Copy Markdown
Contributor Author

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 closed this Nov 6, 2024
tungol deleted the contextvars branch November 7, 2024 21:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL