Skip to content
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

PEP 692: Update specification #3308

Closed
wants to merge 2 commits into from

Conversation

franekmagiera
Copy link
Contributor

@franekmagiera franekmagiera commented Aug 27, 2023

Hey, based on developer feedback I wanted to propose updates to PEP 692's specification. This is supposed to be discussed on discourse first, just wanted to create a PR so that everyone can see the proposed wording. Please do not merge without SC's approval.

Summary of the changes:

  • Type checkers should consider functions typed with Unpack to be equivalent to their explicitly expanded counterparts. Ivan's comment summarizes it pretty well:

There is nothing cumbersome in reducing the PEP to just one paragraph that would explain that Unpack[SomeTD] is a syntactic sugar for (and is considered equivalent to) the expanded signature. This has a number of benefits:

  • This will not add any new unsafety that is not already present for existing uses of TypedDicts in ** contexts. (And type checkers may handle this unsafety in a uniform way, say in mypy we may use existing --extra-checks flag to prohibit some techincally unsafe calls as I mentioned before.)
  • This is actually easy to remember and to reason about.
  • This will allow people who want subtyping between callables to easily achieve this using total=False, which follows from existing rules for expanded callables.

Btw a comment on the last point, subtyping relations are quite counter-intuitive when you think about them in terms of packed TypedDicts. Consider this example:

class A(TypedDict, total=False):
    x: int
class B(TypedDict, total=False):
    x: int
    y: int

def takes_a(**kwargs: Unpack[A]) -> None: ...
def takes_b(**kwargs: Unpack[B]) -> None: ...

if bool():
    takes_a = takes_b  # mypy now says it is OK
else:
    takes_b = takes_a  # mypy now gives an error for this

If you would think in terms of subtyping between TypedDicts, you might conclude the opposite relations, because B is subtype of A, and functions should be contravariant in argument types, right? But obviously takes_a is not a subtype of takes_b because takes_b(x=42, y=0) is a valid call, while takes_a(x=42, y=0) is not a valid one. (You can say this is because mappings are contravariant in keys, but covariant in values etc., but yet again, simple logic of always expanding makes all this unnecessary).

Finally, the simple logic of always expanding is more future-proof. For example, if we would decide to add default types for TypedDicts, it would naturally translate to trailing homogeneous **kwargs in expanded form. While if you treat packed and expanded forms differently (as PEP currently does), it would add many new corner cases.

  • Deleting the Passing kwargs inside a function to another function from the spec - this cannot be reasonably supported, type checkers cannot allow two functions with identical signatures to pass or fail with the same arguments depending on the context.
  • Change is either:
    • To a Draft PEP
    • To an Accepted or Final PEP, without Steering Council approval
    • To fix an editorial issue (markup, typo, link, header, etc)
  • PR title prefixed with PEP number (e.g. PEP 123: Summary of changes)

📚 Documentation preview 📚: https://pep-previews--3308.org.readthedocs.build/

@Rosuav
Copy link
Contributor

Rosuav commented Aug 27, 2023

Cool. If you like, you could make this a draft PR, that would signal that it's not ready for merging.

@franekmagiera franekmagiera changed the title PEP 692: Update specification Draft: PEP 692: Update specification Aug 27, 2023
@franekmagiera franekmagiera marked this pull request as draft August 27, 2023 14:06
@franekmagiera franekmagiera changed the title Draft: PEP 692: Update specification PEP 692: Update specification Aug 27, 2023
pep-0692.rst Outdated Show resolved Hide resolved
@hugovk
Copy link
Member

hugovk commented Nov 24, 2024

This is supposed to be discussed on discourse first, just wanted to create a PR so that everyone can see the proposed wording. Please do not merge without SC's approval.

Triage: 15 months later, where are we up to with this PR? Has the discussion taken place (please share a link if so) and resulted in consensus?

Or is it time to close this? We can always re-open later.

@franekmagiera
Copy link
Contributor Author

Hey,
the discussion has happened. This PEP has been marked as "Final" and it doesn't make sense to update it, instead the typing docs could be updated, but I think there is no need - the proposed update to the PEP focused more on implementation details of the spec. IMO it's fine to leave the docs as is, as it shouldn't confuse the users. So I'll close this PR, feel free to open if you feel differently and sorry for the clutter 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants