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

pep612: add semanal for paramspec #9339

Merged
merged 15 commits into from Aug 30, 2020
Merged

Conversation

hauntsaninja
Copy link
Collaborator

@hauntsaninja hauntsaninja commented Aug 22, 2020

Linking #8645

Taking Jukka's suggestion from #9250 and splitting up the PR so we can merge and review a piece at a time. If you've read what's been pushed to #9250, this is the first bit of it, except I've changed mypy_extensions to typing_extensions since PEP 612 has been accepted.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good, just left a few minor comments. Feel free to merge without waiting for another review round.

(Feel free to ping me on Gitter or send an email if you have a new PR up -- I want to prioritize reviewing PEP 612 related PRs, since this is an important new feature.)

mypy/semanal.py Outdated

In the future, ParamSpec may accept bounds and variance arguments, in which
case more aggressive sharing of code with process_typevar_declaration should be pursued.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: redundant empty line at the end of docstring.


TParams = ParamSpec('TParams')
TP = ParamSpec('?') # E: String argument 1 '?' to ParamSpec(...) does not match variable name 'TP'
TP2: int = ParamSpec('TP2') # E: Cannot declare the type of a parameter specification
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optionally, you can add a test case to test-data/unit/semanal-symtable.test and/or test-data/unit/semanal-types.test (these are not super high signal test cases, but on the other hand they don't require a lot of maintenance).

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Aug 29, 2020

Thanks, made the changes. Had to add two visitors to get the test working. Also got around to recognising typing.ParamSpec, since otherwise testtransform outputs a lot of stuff coming from typing_extensions.

@hauntsaninja hauntsaninja merged commit 0ea510c into python:master Aug 30, 2020
@hauntsaninja hauntsaninja deleted the pep612clean branch August 30, 2020 02:37
JukkaL pushed a commit that referenced this pull request Sep 8, 2020
Linking #8645

Like #9339, much of this is in #9250. However, this PR is a little less self 
contained, in that it causes several future TODOs.

A lot of the changes here are necessitated by changing the type of 
CallableType.variables to Sequence[TypeVarLikeDef]. This is nice, because 
it informs me where I need to make changes in the future / integrates a little 
bit better with existing type var stuff.
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.

None yet

2 participants