Skip to content

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

Merged
AlexWaygood merged 5 commits into
python:mainfrom
sobolevn:issue-88965
Mar 23, 2023
Merged

gh-88965: Fix type substitution of a list of types after initial ParamSpec substitution#102808
AlexWaygood merged 5 commits into
python:mainfrom
sobolevn:issue-88965

Conversation

@sobolevn

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

Copy link
Copy Markdown
Member

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-botbedevere-bot mentioned this pull request Mar 18, 2023
Closed
@sobolevn

sobolevn commented Mar 18, 2023

Copy link
Copy Markdown
MemberAuthor

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

AlexWaygood
AlexWaygood reviewed Mar 19, 2023
View reviewed changes

@AlexWaygoodAlexWaygood left a comment

Copy link
Copy Markdown
Member

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:

Comment threadLib/typing.py Outdated
Comment threadLib/typing.py Outdated
Comment threadLib/test/test_typing.py
@sobolevn

sobolevn commented Mar 22, 2023

Copy link
Copy Markdown
MemberAuthor

@AlexWaygood friendly ping :)

@AlexWaygood

AlexWaygood commented Mar 22, 2023

Copy link
Copy Markdown
Member

@AlexWaygood friendly ping :)

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

@sobolevn

sobolevn commented Mar 22, 2023

Copy link
Copy Markdown
MemberAuthor

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

@AlexWaygoodAlexWaygood left a comment

Copy link
Copy Markdown
Member

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 reviewed Mar 23, 2023
View reviewed changes
Comment threadMisc/NEWS.d/next/Library/2023-03-18-14-59-21.gh-issue-88965.kA70Km.rst Outdated
@AlexWaygoodAlexWaygood added type-bug An unexpected behavior, bug, or error needs backport to 3.10 only security fixes stdlib Standard Library Python modules in the Lib/ directory topic-typing needs backport to 3.11 only security fixes labels Mar 23, 2023
@AlexWaygoodAlexWaygood self-assigned this Mar 23, 2023
@AlexWaygood
AlexWaygood merged commit adb0621 into python:mainMar 23, 2023
@miss-islington

miss-islington commented Mar 23, 2023

Copy link
Copy Markdown
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

miss-islington commented Mar 23, 2023

Copy link
Copy Markdown
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

@AlexWaygoodAlexWaygood removed the needs backport to 3.11 only security fixes label Mar 23, 2023
@miss-islington

miss-islington commented Mar 23, 2023

Copy link
Copy Markdown
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

@AlexWaygoodAlexWaygood added the needs backport to 3.11 only security fixes label Mar 23, 2023
@miss-islington

miss-islington commented Mar 23, 2023

Copy link
Copy Markdown
Contributor

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

@miss-islington

miss-islington commented Mar 23, 2023

Copy link
Copy Markdown
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

@AlexWaygoodAlexWaygood 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

miss-islington commented Mar 23, 2023

Copy link
Copy Markdown
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

bedevere-bot commented Mar 23, 2023

Copy link
Copy Markdown

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

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

AlexWaygood commented Mar 23, 2023•
edited
Loading

Copy link
Copy Markdown
Member

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

gvanrossum commented Mar 23, 2023

Copy link
Copy Markdown
Member

Let’s leave 3.10 alone.

@AlexWaygoodAlexWaygood removed the needs backport to 3.10 only security fixes label Mar 23, 2023
@sobolevn

sobolevn commented Mar 23, 2023

Copy link
Copy Markdown
MemberAuthor

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

sobolevn commented Mar 24, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

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

@AlexWaygood

AlexWaygood commented Mar 24, 2023

Copy link
Copy Markdown
Member

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

@sobolevnsobolevn 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygoodAlexWaygoodAlexWaygood approved these changes
@gvanrossumgvanrossumAwaiting requested review from gvanrossum
@Fidget-SpinnerFidget-SpinnerAwaiting requested review from Fidget-Spinner
@JelleZijlstraJelleZijlstraAwaiting requested review from JelleZijlstraJelleZijlstra is a code owner

Assignees

@AlexWaygoodAlexWaygood

Labels

stdlibStandard Library Python modules in the Lib/ directorytopic-typingtype-bugAn 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