Skip to content

gh-88965: Fix type substitution of a list of types after initial ParamSpec substitution #102808

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Mar 23, 2023
Merged

gh-88965: Fix type substitution of a list of types after initial ParamSpec substitution #102808

merged 5 commits into from
Mar 23, 2023

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Mar 18, 2023 •
edited by bedevere-bot
Loading

I've tried to make tests as complete as I can imagine. But, extra cases and suggestions are welcome! ;)

This change has two parts:

  1. Collecting type parameters from arguments is changed. Now, we analyze the contents of passes tuples
  2. Next, we need to substitute nested tuple. I've reused the existing logic here, but I had to create a method for it

@bedevere-bot bedevere-bot mentioned this pull request Mar 18, 2023
Closed
@sobolevn
Copy link
Member Author

@AlexWaygood you might be interested, since you've originally asked to take a look :)

AlexWaygood
AlexWaygood reviewed Mar 19, 2023
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

I think this is the correct solution -- thanks so much for taking a look at this! I have a couple of points about code readability, and a suggestion about the tests:

@sobolevn
Copy link
Member Author

@AlexWaygood friendly ping :)

@AlexWaygood
Copy link
Member

@AlexWaygood friendly ping :)

I haven't forgotten! It's only been two days 😄

@sobolevn
Copy link
Member Author

It's only been two days

Oh wow, the time is a very strange thing!
Sorry, you became a victim of my regular pushing-PRs-forward process :)

AlexWaygood
AlexWaygood approved these changes Mar 23, 2023
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Looks great to me. I have some ideas about how to clean up the tuple special-casing, but I think that should wait for another PR, as it might not be so backportable.

@AlexWaygood AlexWaygood added type-bug An unexpected behavior, bug, or error needs backport to 3.10 only security fixes stdlib Python modules in the Lib dir topic-typing needs backport to 3.11 only security fixes labels Mar 23, 2023
@AlexWaygood AlexWaygood self-assigned this Mar 23, 2023
@AlexWaygood AlexWaygood merged commit adb0621 into python:main Mar 23, 2023
@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @AlexWaygood for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @sobolevn and @AlexWaygood, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker adb0621652f489033b9db8d3949564c9fe545c1d 3.11

@AlexWaygood AlexWaygood removed the needs backport to 3.11 only security fixes label Mar 23, 2023
@miss-islington
Copy link
Contributor

Sorry, @sobolevn and @AlexWaygood, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker adb0621652f489033b9db8d3949564c9fe545c1d 3.10

@AlexWaygood AlexWaygood added the needs backport to 3.11 only security fixes label Mar 23, 2023
@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @AlexWaygood for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @sobolevn and @AlexWaygood, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker adb0621652f489033b9db8d3949564c9fe545c1d 3.11

@AlexWaygood AlexWaygood added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Mar 23, 2023
@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @AlexWaygood for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 23, 2023
@sobolevn @AlexWaygood @miss-islington
pythongh-88965: typing: fix type substitution of a list of types afte…
3d006ac 
…r initial `ParamSpec` substitution (pythonGH-102808)

Previously, this used to fail:

```py
from typing import *
T = TypeVar("T")
P = ParamSpec("P")

class X(Generic[P]):
    f: Callable[P, int]

Y = X[[int, T]]
Z = Y[str]
```

(cherry picked from commit adb0621)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@bedevere-bot
Copy link

GH-102957 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Mar 23, 2023
@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 23, 2023 •
edited
Loading

I think backporting this might be quite complex, due to the fact that TypeVarTuple was new in 3.11, meaning a lot of the machinery around substitution changed significantly in 3.11. I glanced at the merge conflicts and they seem nontrivial to resolve.

If you'd like to give it a stab @sobolevn, I'll happily review a backport PR -- but I'm also happy to just leave it unfixed on 3.10.

@gvanrossum
Copy link
Member

Let’s leave 3.10 alone.

@AlexWaygood AlexWaygood removed the needs backport to 3.10 only security fixes label Mar 23, 2023
@sobolevn
Copy link
Member Author

Let’s leave 3.10 alone.

Ok, I won't bother then.

Thanks everyone! 👍

miss-islington added a commit that referenced this pull request Mar 23, 2023
@miss-islington @sobolevn @AlexWaygood
gh-88965: typing: fix type substitution of a list of types after init…
1645a40 
…ial `ParamSpec` substitution (GH-102808)

Previously, this used to fail:

```py
from typing import *
T = TypeVar("T")
P = ParamSpec("P")

class X(Generic[P]):
    f: Callable[P, int]

Y = X[[int, T]]
Z = Y[str]
```

(cherry picked from commit adb0621)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@sobolevn
Copy link
Member Author

sobolevn commented Mar 24, 2023 •
edited
Loading

@AlexWaygood btw, do you have any ideas of issues I can work on? I am out of my backlog :(

@AlexWaygood
Copy link
Member

I have one in the back of my head that I'll try to write up today ;)

@sobolevn sobolevn mentioned this pull request Mar 27, 2023
Closed
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
@sobolevn @AlexWaygood @Fidget-Spinner
pythongh-88965: typing: fix type substitution of a list of types afte…
67a48e5 
…r initial `ParamSpec` substitution (python#102808)

Previously, this used to fail:

```py
from typing import *
T = TypeVar("T")
P = ParamSpec("P")

class X(Generic[P]):
    f: Callable[P, int]

Y = X[[int, T]]
Z = Y[str]
```

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@sobolevn @AlexWaygood @warsaw
pythongh-88965: typing: fix type substitution of a list of types afte…
a6097ae 
…r initial `ParamSpec` substitution (python#102808)

Previously, this used to fail:

```py
from typing import *
T = TypeVar("T")
P = ParamSpec("P")

class X(Generic[P]):
    f: Callable[P, int]

Y = X[[int, T]]
Z = Y[str]
```

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

@Fidget-Spinner Fidget-Spinner Awaiting requested review from Fidget-Spinner

@JelleZijlstra JelleZijlstra Awaiting requested review from JelleZijlstra JelleZijlstra is a code owner

Assignees

@AlexWaygood AlexWaygood

Labels
stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sobolevn @AlexWaygood @miss-islington @bedevere-bot @gvanrossum