cpython icon indicating copy to clipboard operation
cpython copied to clipboard

GH-87390: Add remaining tests for PEP 646

Open mrahtz opened this issue 1 year ago • 1 comments

This PR fixes the last few things from the PEP 646 implementation checklist at https://github.com/python/cpython/issues/87390#issuecomment-1104419757. Specifically:

  • Fix a weird hack we used for star-unpacking in test_genericalias.py before the star-unpacking grammar change was merged
  • Add some more substitution tests (eg [tuple[*Ts, T][str, *tuple[int, ...]])
  • Fix some outdated comments
  • Add tests to typing.py for *ed stuff (again, now that the grammar change is merged)
  • Add tests for get_origin on unpacked stuff

A couple notes:

  • If we were being really exhaustive, we would test the cross product of tuple, Tuple and *, Unpack, for a total of 4 versions of each test, but that would be reeeaaally verbose, so I've gone with just testing the 'new-style' cases (* and tuple) and the 'old-style' cases (Unpack and Tuple).
  • get_origin currently gives different behaviour for *tuple[int] and Unpack[tuple[int]]: tuple in the former case, Unpack in the latter. This is a bit inconsistent, but it seems like the least surprising thing from a user's perspective: if I saw Unpack[something], I'd intuitively expected the origin to be Unpack; and if I saw *tuple[something], I'd expect the origin to be tuple.
  • Issue: gh-87390

mrahtz avatar Oct 14 '22 17:10 mrahtz

Oops, good catch - I think I must have accidentally committed my debug changes, because I found some other things that needed removing. Have fixed them and confirmed that CI is actually passing now :)

mrahtz avatar Oct 15 '22 08:10 mrahtz

@JelleZijlstra Are you happy with these tests? Then you can merge them.

gvanrossum avatar Oct 24 '22 16:10 gvanrossum

Probably, I'll take another look tonight and merge if I'm happy.

JelleZijlstra avatar Oct 24 '22 16:10 JelleZijlstra

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

miss-islington avatar Oct 25 '22 14:10 miss-islington

Sorry, @mrahtz and @JelleZijlstra, I could not cleanly backport this to 3.11 due to a conflict. Please backport using cherry_picker on command line. cherry_picker cb95cc24efecf32ad035318867b065a2b042d25a 3.11

miss-islington avatar Oct 25 '22 14:10 miss-islington

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

bedevere-bot avatar Oct 25 '22 14:10 bedevere-bot