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

Add foundation for TypeVar defaults (PEP 696) #14872

Merged
merged 3 commits into from
May 29, 2023

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Mar 11, 2023

Start implementing PEP 696 TypeVar defaults. This PR

  • Adds a default parameter to TypeVarLikeExpr and TypeVarLikeType.
  • Updates most visitors to account for the new default parameter.
  • Update existing calls to add value for default => AnyType(TypeOfAny.from_omitted_generics).

A followup PR will update the semantic analyzer and add basic tests for TypeVar, ParamSpec, and TypeVarTuple calls with a default argument. -> #14873

Ref #14851

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@hauntsaninja hauntsaninja requested a review from JukkaL May 2, 2023 18:22
mypy/semanal.py Outdated
# Similar to regular (user defined) type variables.
self.process_placeholder(
None,
"Self upper bound",
"Self upper bound or default",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe better to have a separate message for the two cases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this particular instance I believe it isn't even necessary to check it at all. For Self the default is always set to AnyType(TypeOfAny.from_omitted_generics) and thus can't include a placeholder. I've split the calls in process_typevar_declaration though.

Also noticed that I was missing these checks in process_paramspec_declaration and process_typevartuple_declaration which I added now.

@github-actions

This comment has been minimized.

@cdce8p cdce8p mentioned this pull request May 10, 2023
@github-actions

This comment has been minimized.

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.

Looks good, one suggestion

mypy/nodes.py Show resolved Hide resolved
@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@cdce8p
Copy link
Collaborator Author

cdce8p commented May 29, 2023

Thanks @JelleZijlstra! I'll take a closer look at my followup PR tomorrow.
Would you mind creating a new label, something like topic-pep-696? That would help keep track of PRs and upcoming issues.

@JelleZijlstra
Copy link
Member

Done: topic-pep-696 TypeVar defaults . I also just fixed the merge on #14873 (possibly incorrectly, there's some self-check failures) and added some comments.

@JelleZijlstra JelleZijlstra added the topic-pep-696 TypeVar defaults label May 29, 2023
@cdce8p cdce8p deleted the TypeVar-01-foundation branch May 29, 2023 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-pep-696 TypeVar defaults
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants