gh-87390: Add tests demonstrating current type variable substitution behaviour by mrahtz · Pull Request #32341 · python/cpython · GitHub

Here's a reference of what we currently do and don't allow in terms of type variable substitution in aliases. I've marked the cases that I think the current implementation is incorrect on.

Ideally I'd like to merge this as-is, even with the test cases whose expected results aren't what we think they should be - I think it'll make our job implementing the remaining fixes easier if we can just twiddle one line of an existing test case rather than having to remember to copy-paste the test case that each change fixes.

Some cases I'm unsure about are:

tuple[T][*tuple[int, ...]]. My understanding is that when it comes to tuple[int, ...], the current behaviour of type checkers is to "Try to find a way to make it work" - I think this should be tuple[int]?
class C(Generic[T1, T2]): ...; C[*tuple[int, ...], *tuple[str, ...]]. By similar reasoning, this should work - but should it be C[int, int], C[int, str], or C[str, str]?

@pradeep90 What do you think?

https://bugs.python.org/issue47006

This reverts commit 526d9e8.

Thanks, this is a really helpful list.

All commit authors signed the Contributor License Agreement.

Re @JelleZijlstra's suggestion (if I understood right?) to leave unpacked tuples unsimplified across the board: I just realised this means we might have to ease up on checks for the right number of arguments across the board.

For example:

tuple[T1, T2][*tuple[int, str]]

This should be valid, but I think at the moment it would trigger a TypeError, because it looks like we're passing only a single argument *tuple[int, str] to an alias that expects two arguments.

I guess easing up on type argument number checks should be fine though? The alternative would be to have some fancy logic that tries to detect how many type arguments something like *tuple[int, str] actually counts for, but that seems like it could get messy.

Actually, having thought about it more over the past few days, I don't think leaving unpacked tuples unsimplified in all case will work. For example, if we did:

Alias = tuple[T1, bool, T2]
Alias[*tuple[int, str]]  # Should be tuple[int, bool, str]

There's no way we could properly evaluate the result of that unless we did unpack *tuple[int, str]. I've added another test case for this scenario.

Having said that, Jelle, I'm still bearing in mind your comment:

'generic[T]', '[*tuple_type[int, ...]]', 'generic[*tuple_type[int, ...]]' # Should be generic[int]?

I'd rather leave this as is. If we simplify it, we make it impossible for runtime introspection to even tell the difference.

I do agree that for the particular case of *tuple[int, ...] we should leave it unsimplified. I think the only alias involving *tuple[int, ...] that should be valid at runtime for now is generic[*Ts][*tuple[int, ...]].

I'll update the tests accordingly.

(Damn, GitHub doesn't make it obvious at all whether you've submitted a review or not. I thought I'd submitted this days ago.)

For what it's worth, there won't be an observable difference from the type checking point of view. For example, if someone tries to pass in this instantiated alias C[*tuple[int, ...]] to a function that expects C[A, *Rs], they will still get A=int, Rs=tuple[int, ...].

So, we have some freedom in what to do here. If getting C[int, *tuple[int, ...]] is hard, it might be ok to keep C[*tuple[int, ...]]

I think this is a case I updated since your review. The vibe I'm getting from @JelleZijlstra is that the kind of logic complexity he'd be happy with (correct me if I'm wrong, @JelleZijlstra) wouldn't be up to dealing with C[T, *Ts][*tuple[int, ...]] - either figuring out the 'right' answer of C[int, *tuple[int, ...]], or accepting it as valid and giving the mostly-right answer of C[*tuple[int, ...]].

In fact, according to the updated tentative spec at #91162, we'd disallow this altogether - we wouldn't allow any cases where *tuple[int, ...] has to be split among multiple things.

I came across a case in the typing-extensions tests that is not covered here yet:

from typing import *
Ts = TypeVarTuple('Ts')
T1 = TypeVar('T')
T2 = TypeVar('T')
class C(Generic[T1, T2, Unpack[Ts]]): pass
with self.assertRaises(TypeError):
    C[int]

typing.py currently accepts this without errors.

- Defer to the PEP 646 implementation in typing.py on 3.11 - Adjust some tests accordingly. Noted a bug in python/cpython#32341 (comment) - typing._type_check() is more lenient in 3.11 and no longer rejects ints - The representation of the empty tuple type changed Tests pass for me on a 3.11 build from today now.

… is non-variadic

[@JelleZijlstra]

I came across a case in the typing-extensions tests that is not covered here yet:
from typing import *
Ts = TypeVarTuple('Ts')
T1 = TypeVar('T')
T2 = TypeVar('T')
class C(Generic[T1, T2, Unpack[Ts]]): pass
with self.assertRaises(TypeError):
    C[int]
typing.py currently accepts this without errors.

For the time being, this intentional - we decided not to check arity at runtime for variadics (https://github.com/python/cpython/blob/main/Lib/typing.py#L1331).

- Defer to the PEP 646 implementation in typing.py on 3.11 - Adjust some tests accordingly. Noted a bug in python/cpython#32341 (comment) - typing._type_check() is more lenient in 3.11 and no longer rejects ints - The representation of the empty tuple type changed Tests pass for me on a 3.11 build from today now.

…id arguments for a *Ts

…o* now get a TypeError

…ython#91162

Going to merge this now so we have the test framework. We'll likely adjust some of the cases though.

- Defer to the PEP 646 implementation in typing.py on 3.11 - Adjust some tests accordingly. Noted a bug in python/cpython#32341 (comment) - typing._type_check() is more lenient in 3.11 and no longer rejects ints - The representation of the empty tuple type changed Tests pass for me on a 3.11 build from today now.

Add integration tests for type variable substitution

f508d12

bedevere-bot added the awaiting review label Apr 5, 2022

the-knights-who-say-ni added the CLA signed label Apr 5, 2022

bedevere-bot added the tests Tests in the Lib/test dir label Apr 5, 2022

gvanrossum reviewed Apr 6, 2022

View reviewed changes

Comment thread Lib/test/test_typing.py Outdated

mrahtz added 6 commits April 8, 2022 21:56

Add tests for list, dict and tuple

ea104b9

Add tests for Tuple

526d9e8

Revert "Add tests for Tuple"

0f022db

This reverts commit 526d9e8.

Make it easier to tests all tuple, list and dict types

cc7bf8c

TypeVarSubstitutionTests -> GenericAliasSubstitutionTests

458b5dd

Remove question marks from cases I'm actually pretty sure about

0c8ba38

mrahtz marked this pull request as ready for review April 10, 2022 09:40

mrahtz requested review from Fidget-Spinner and JelleZijlstra as code owners April 10, 2022 09:40

JelleZijlstra reviewed Apr 11, 2022

View reviewed changes

Comment thread Lib/test/test_typing.py Outdated

Comment thread Lib/test/test_typing.py Outdated

Comment thread Lib/test/test_typing.py Outdated

Note that list[] should only take one argument

db24e70

mrahtz added 2 commits April 11, 2022 19:23

Leave unpacked tuples unsimplified

15a4809

Add generic[*Ts][*tuple[*Ts]]

0d7d2eb

mrahtz changed the title bpo-47006: Add tests demonstrating current type variable substitution behaviour gh-87390: Add tests demonstrating current type variable substitution behaviour Apr 11, 2022

Update comments based on decision to leave unpacked tuples unsimplified

ab01c8a

JelleZijlstra mentioned this pull request Apr 11, 2022

PEP 646: Decide on substitution behavior #91162

Closed

AlexWaygood added the topic-typing label Apr 13, 2022

Add tests for generic[T1, bool, T2][*tuple[int, str]]

c4fe922

Update comments based on decision to unpack *some* tuples

d55b8fb

pradeep90 approved these changes Apr 15, 2022

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Apr 15, 2022

pradeep90 reviewed Apr 15, 2022

View reviewed changes

Comment thread Lib/test/test_typing.py Outdated

JelleZijlstra mentioned this pull request Apr 15, 2022

Fix tests on Python 3.11 python/typing#1139

Merged

mrahtz added 4 commits April 16, 2022 17:45

Note that generic[*tuple[()]] should raise TypeError

e858fc6

Note that C[T, *tuple[int, ...]][int] raises TypeError only because C…

3bc7d26

… is non-variadic

State that multiple unpackings should be allowed where possible

c980e4f

Add test for generic[T1, *tuple[int, ...], T2][str, bool, float]

95b7210

Update comments according to revised tentative spec at python#91162

96394b1

mrahtz added 6 commits April 18, 2022 08:14

Update expected comments to account for unpacked types only being val…

ed9a557

…id arguments for a *Ts

Merge branch 'main' into subst-tests

faa9fc3

Update to reflect results after merging latest main

9947fc0

Update ALL tests to reflect results after merging latest main

12496fc

Remove comments saying we should get a TypeError in cases where we *d…

db9af53

…o* now get a TypeError

Update comments on expected results based on latest tentative spec at p…

c5b5a63

…ython#91162

JelleZijlstra added the skip news label Apr 29, 2022

JelleZijlstra self-assigned this Apr 29, 2022

JelleZijlstra merged commit f665616 into python:main Apr 29, 2022

bedevere-bot removed the awaiting core review label Apr 29, 2022

mrahtz deleted the subst-tests branch April 29, 2022 20:15

mrahtz mentioned this pull request May 7, 2022

gh-91162: Fix substitution of unpacked tuples in generic aliases #92335

Merged

+                          ('tuple[T, *Ts]',                          '[int, str, bool]',                               'TypeError'),  # Should be tuple[int, str, bool]
+                          ('Tuple[T, *Ts]',                          '[int, str, bool]',                               'Tuple[int, str, bool]'),
+                          ('C[T, *Ts]',                              '[*tuple_type[int, ...]]',                        'C[*tuple_type[int, ...]]'),  # Should be C[int, *tuple_type[int, ...]]

gh-87390: Add tests demonstrating current type variable substitution behaviour by mrahtz · Pull Request #32341 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-87390: Add tests demonstrating current type variable substitution behaviour - #32341

gh-87390: Add tests demonstrating current type variable substitution behaviour#32341JelleZijlstra merged 24 commits intopython:mainpython/cpython:mainfrom mrahtz:subst-testsmrahtz/cpython:subst-testsCopy head branch name to clipboard

Conversation

mrahtz commented Apr 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Apr 11, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrahtz commented Apr 11, 2022

Uh oh!

mrahtz commented Apr 15, 2022

Uh oh!

pradeep90 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pradeep90 Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

mrahtz Apr 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrahtz Apr 16, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JelleZijlstra commented Apr 15, 2022

Uh oh!

mrahtz commented Apr 16, 2022

Uh oh!

JelleZijlstra commented Apr 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

gh-87390: Add tests demonstrating current type variable substitution behaviour#32341
JelleZijlstra merged 24 commits into
python:mainfrom
mrahtz:subst-tests

mrahtz commented Apr 5, 2022 •
edited
Loading

ghost commented Apr 11, 2022 •
edited by ghost
Loading

pradeep90 left a comment •
edited
Loading

mrahtz Apr 16, 2022 •
edited
Loading