Skip to content

Address feedback from JelleZijlstra#4

Merged
alicederyn merged 1 commit intopep.typedmappingfrom
jelle-zijlstra-revisions
Feb 13, 2023
Merged

Address feedback from JelleZijlstra#4
alicederyn merged 1 commit intopep.typedmappingfrom
jelle-zijlstra-revisions

Conversation

@alicederyn
Copy link
Copy Markdown
Owner

Define what TypedMapping does support, not just what it doesn't, to clarify edge cases like |.

Define what TypedMapping does support, not just what it doesn't, to clarify edge cases like ``|``.
@alicederyn alicederyn changed the title Revisions suggested by JelleZijlstra Address feedback from JelleZijlstra Feb 8, 2023
Comment thread pep-0705.rst
* No mutator methods (``__setitem__``, ``__delitem__``, ``update``, etc.) will be generated
* A class definition defines a ``TypedMapping`` protocol if and only if ``TypedMapping`` appears directly in its class bases
* Subclasses can narrow field types, in the same manner as other protocols
Notable similarities to ``TypedDict``:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This still reads like a tutorial, not a specification. I would frame it more around collections.abc.Mapping: TypedMapping supports the same operations as Mapping[str, object], except that the defined keys produce more precise types around methods like get and __getitem__.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Is this different from my change to line 96?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's not! Sorry for missing that line in the diff.

Comment thread pep-0705.rst
Author: Alice Purcell <alicederyn@gmail.com>
Sponsor: Pablo Galindo <pablogsal@gmail.com>
Discussions-To: https://mail.python.org/archives/list/python-dev@python.org/thread/2P26R4VH2ZCNNNOQCBZWEM4RNF35OXOW/
Discussions-To: pending
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just FYI, the linter will go red on this and flag it, but perhaps that's intentional as a reminder to update it immediately prior to merging (if that's your plan).

@alicederyn alicederyn merged commit f425e49 into pep.typedmapping Feb 13, 2023
@alicederyn alicederyn deleted the jelle-zijlstra-revisions branch February 13, 2023 08:14
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