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 747: Revised TypeForm specification #3929

Merged
merged 14 commits into from
Sep 15, 2024

Conversation

erictraut
Copy link
Contributor

@erictraut erictraut commented Sep 1, 2024

This is updated version of draft PEP 747. It attempts to clarify the terminology, and it changes the design to eliminate limitations and reduce edge cases and backward compatibility issues.


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

peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
erictraut and others added 3 commits September 2, 2024 08:58
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Copy link
Contributor

@davidfstr davidfstr left a comment

Choose a reason for hiding this comment

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

Lots of great work here Eric! I've left some comments below.

peps/pep-0747.rst Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great.

My main area of concern is the mandated "dual evaluation", which feels inconsistent with how evaluation normally works. I would prefer to have behavior similar to what we do for TypedDict:

from typing import TypedDict

class TD(TypedDict):
    a: int

x = {"a": 1}
reveal_type(x)  # dict[str, int]
y: TD = {"a": 1}
reveal_type(y)  # TD

So expressions are by default evaluated as they are now, but in a context where a TypeForm is expected (e.g., when assigning to a variable of type TypeForm), we infer a TypeForm type.

peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
peps/pep-0747.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@davidfstr davidfstr left a comment

Choose a reason for hiding this comment

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

The revised approach - using only the new "TypeForm interpretation" when an explicit TypeForm annotation is used on the receiving variable/parameter - looks good. It avoids the "duality of types" seen before. It also appears to sidestep any reliance on an intersection type (&) which avoids entangling with future work in that area of the type system. 👍

I've left one minor comment suggesting reinstating a code example to make some of the text more clear.

I'll make a post in discuss.python.org inviting further feedback on this PR shortly.

peps/pep-0747.rst Outdated Show resolved Hide resolved
@JelleZijlstra JelleZijlstra merged commit 02a7034 into python:main Sep 15, 2024
6 checks passed
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.

4 participants