bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing #23702

Fidget-Spinner · 2020-12-08T17:21:09Z

Changes:

Documentation will be another patch.

https://bugs.python.org/issue41559

gvanrossum

I'm sorry, I didn't get to the end of this review (not by far), but I have a few comments. Main thing though is, did you look at all the examples in PEP 612? It seems that for user-defined generic classes that have a ParamSpec parameter, things like X[int, P] are actually valid.

Maybe this indirectly also answers your question: I think that the ParamSpec should be included in __parameters__.

Also, I wonder if we could do less type checking at runtime rather than trying to be strict? I desperately want typing.py to shrink, not grow... :-)

Lib/typing.py

Lib/test/test_typing.py

Fidget-Spinner · 2020-12-09T23:40:53Z

Points taken. Sorry X[int, P] slipped my mind since I hadn't fixed __parameters__ yet.

D'oh, I just realised I answered my own question since we need __parameters__ to implement X[int, P] in the first place.

Co-Authored-By: Guido van Rossum <gvanrossum@users.noreply.github.com>

also reduced runtime checks

gvanrossum · 2020-12-11T23:35:19Z

Let's wait until #23060 has landed and then see what adjustments this might need.

gvanrossum · 2020-12-23T01:00:20Z

Finally looking at this...

Actually I just realized PEP 612 doesn't explicitly say that ParamSpec should support TypeVar-like substitution. Maybe I over complicated things a little by allowing that in types.GenericAlias (just for collections.abc.Callable)?

Yeah, ParamSpec is super special, and you can't do anything with it that isn't explicitly mentioned in the PEP. So it's debatable whether Callable[P, int] should even have P in its __parameters__. Then again, for the simple case A = Callable[P, int] there's no reason why we couldn't allow A[[str, str]] and get Callable[[str, str], int] as the result. But type checkers wouldn't know what to do with that -- ParamSpec is a magic cookie.

So let's not worry about those clearly wrong cases.

gvanrossum · 2020-12-23T01:24:15Z

Sorry, submitted too soon. (UPDATE: Not, I made a mistake -- or GitHub did. :-)

gvanrossum

Excellent work! Thanks for your perseverance. My feedback is mostly simple improvements.

Lib/_collections_abc.py

Lib/test/test_typing.py

Lib/typing.py

Lib/_collections_abc.py

Objects/genericaliasobject.c

gvanrossum · 2020-12-23T02:11:31Z

Objects/genericaliasobject.c

+            }
+            // convert all the lists inside args to tuples to help
+            // with caching in other libaries if substituting a ParamSpec
+            if (PyList_CheckExact(subst) && is_paramspec(arg)) {


Why only do this for ParamSpec?

I'm afraid of existing code relying on substituting typevars in genericalias with lists, then expecting that list to be inside __args__. Although in Python 3.9 that should be invalid, and the IDE/type checker should warn the users about that already.

If I just convert list to tuples for all cases, the C code will become way simpler :).

There can't be much code yet relying about anything for GenericAlias -- it's new in 3.9. We could backport that particular behavior to 3.9.3 to ensure there won't be any more code relying on it. :-)

Objects/genericaliasobject.c

Co-Authored-By: Guido van Rossum <gvanrossum@users.noreply.github.com>

…by default

gvanrossum

Re: the 3.9 issues that you discovered: maybe create a PR for those first, land that in master and 3.9 branch, then merge master into this PR?

Lib/_collections_abc.py

Fidget-Spinner · 2020-12-24T02:44:33Z

Guido, thank you so much for all the time you've put into reviewing this PR! I learnt quite a bit about the typing module in the process :).

Re: the 3.9 issues that you discovered: maybe create a PR for those first, land that in master and 3.9 branch, then merge master into this PR?

Yeah I agree, I created #23915 for that. Once that one lands, I'll merge the changes in along with some of your suggestions.

Lib/_collections_abc.py

gvanrossum · 2020-12-24T03:08:17Z

I think it’s ready!

gvanrossum · 2020-12-24T04:21:12Z

@Fidget-Spinner Let me know if you agree.

Fidget-Spinner · 2020-12-24T04:22:32Z

@Fidget-Spinner Let me know if you agree.

Yes! Sorry, I thought you were going to merge it in so I was waiting too 😆 .

gvanrossum · 2020-12-24T04:33:39Z

I figured that was the case. :-)

bedevere-bot · 2020-12-24T04:33:52Z

@gvanrossum: Please replace # with GH- in the commit message next time. Thanks!

Fidget-Spinner · 2020-12-24T04:41:23Z

@gvanrossum Oh dang I knew I had a hunch I missed something :/. I forgot to convert all lists to tuples in ga_new/setup_ga too (I did it only for getitem and co.). Well on the bright side, having that as a separate PR makes backporting to 3.9 easier if anything :), and anyways lists aren't valid in genericalias other than the Callable case (which we already have covered), so we won't have anything broken even without that in right now.

Fidget-Spinner · 2020-12-24T04:42:19Z

I'll fix that some other time, right now I'm celebrating 🥳

python#23702)

Fidget-Spinner added 2 commits December 9, 2020 01:04

Add ParamSpec and Concatenate

219b4ee

support ParamSpec in generics

a1c0d0a

the-knights-who-say-ni added the CLA signed label Dec 8, 2020

bedevere-bot added the awaiting review label Dec 8, 2020

Fidget-Spinner added 2 commits December 10, 2020 00:07

Add typing tests, disallow Concatenate in other types

7b3beab

Add news

5dd3b44

Fidget-Spinner marked this pull request as ready for review December 9, 2020 16:12

Fidget-Spinner requested review from gvanrossum and ilevkivskyi as code owners December 9, 2020 16:12

gvanrossum reviewed Dec 9, 2020

View reviewed changes

Fidget-Spinner marked this pull request as draft December 10, 2020 13:42

Fidget-Spinner and others added 4 commits December 10, 2020 22:34

Address some of Guido's review comments

59c0b20

Co-Authored-By: Guido van Rossum <gvanrossum@users.noreply.github.com>

remove extraneous empty lines

4c381b3

Support ParamSpec in __parameters__ of typing and builtin GenericAlias

b36b62d

also reduced runtime checks

add tests for user defined generics

d09d088

Fidget-Spinner added 6 commits December 14, 2020 22:53

Merge remote-tracking branch 'upstream/master' into pep612

0a19f34

cast list to tuple done, loosened type checks for Generic

9727e2a

loosen generics, allow typevar-like subst, flatten out args if Callable

cc7fc1c

fix whitespace issue, cast list to tuple for types.GenericAlias

3e67f23

convert list to tuples if substituting paramspecs in types.GenericAlias

d9baa1a

done! flattened __args__ in substitutions for collections.abc.Callable

c4155b6

Fidget-Spinner marked this pull request as ready for review December 16, 2020 09:45

Fidget-Spinner requested a review from rhettinger as a code owner December 16, 2020 09:45

Fidget-Spinner added 3 commits December 16, 2020 22:02

fix repr problems, add repr tests

2dbf861

Add another test for multiple chaining

87c2d19

fix typo

2b09de6

Fidget-Spinner marked this pull request as draft December 17, 2020 15:39

Clean up some comments

d980702

remove stray whitespace

d6f777c

gvanrossum reviewed Dec 23, 2020

View reviewed changes

Fidget-Spinner and others added 4 commits December 23, 2020 13:06

Address nearly all of Guido's reviews

9a8176b

Co-Authored-By: Guido van Rossum <gvanrossum@users.noreply.github.com>

more reviews; fix some docstrings, clean up code, cast list to tuple …

fa06838

…by default

remove uneeded tests copied over from typevar

b8672cd

remove unused variable

51a463c

gvanrossum reviewed Dec 23, 2020

View reviewed changes

Lib/_collections_abc.py Outdated Show resolved Hide resolved

Fidget-Spinner added 2 commits December 24, 2020 10:57

Merge remote-tracking branch 'upstream/master' into pep612

6d5b754

merge length checking into _has_special_args too

c05d5d7

gvanrossum approved these changes Dec 24, 2020

View reviewed changes

Lib/_collections_abc.py Outdated Show resolved Hide resolved

bedevere-bot added awaiting merge and removed awaiting review labels Dec 24, 2020

Update Lib/_collections_abc.py

c49ba30

Fidget-Spinner mentioned this pull request Dec 24, 2020

PEP 612: Clean up code examples, link to CPython implementation python/peps#1745

Merged

gvanrossum merged commit 73607be into python:master Dec 24, 2020

bedevere-bot removed the awaiting merge label Dec 24, 2020

Fidget-Spinner deleted the pep612 branch December 24, 2020 09:19

Fidget-Spinner mentioned this pull request Dec 30, 2020

bpo-41559: Documentation for PEP 612 #24000

Merged

Fidget-Spinner mentioned this pull request Feb 14, 2021

bpo-43224: Initial implementation of PEP 646 in typing.py #24527

Closed

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021

bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing (

41127ae

python#23702)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing #23702

bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing #23702

Fidget-Spinner commented Dec 8, 2020 •

edited

Loading

gvanrossum left a comment

Fidget-Spinner commented Dec 9, 2020

gvanrossum commented Dec 11, 2020

gvanrossum commented Dec 23, 2020

gvanrossum commented Dec 23, 2020 •

edited

Loading

gvanrossum left a comment

gvanrossum Dec 23, 2020

Fidget-Spinner Dec 23, 2020

gvanrossum Dec 23, 2020

gvanrossum left a comment

Fidget-Spinner commented Dec 24, 2020 •

edited

Loading

gvanrossum commented Dec 24, 2020

gvanrossum commented Dec 24, 2020

Fidget-Spinner commented Dec 24, 2020

gvanrossum commented Dec 24, 2020

bedevere-bot commented Dec 24, 2020

Fidget-Spinner commented Dec 24, 2020

Fidget-Spinner commented Dec 24, 2020

bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing #23702

bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing #23702

Conversation

Fidget-Spinner commented Dec 8, 2020 • edited Loading

gvanrossum left a comment

Choose a reason for hiding this comment

Fidget-Spinner commented Dec 9, 2020

gvanrossum commented Dec 11, 2020

gvanrossum commented Dec 23, 2020

gvanrossum commented Dec 23, 2020 • edited Loading

gvanrossum left a comment

Choose a reason for hiding this comment

gvanrossum Dec 23, 2020

Choose a reason for hiding this comment

Fidget-Spinner Dec 23, 2020

Choose a reason for hiding this comment

gvanrossum Dec 23, 2020

Choose a reason for hiding this comment

gvanrossum left a comment

Choose a reason for hiding this comment

Fidget-Spinner commented Dec 24, 2020 • edited Loading

gvanrossum commented Dec 24, 2020

gvanrossum commented Dec 24, 2020

Fidget-Spinner commented Dec 24, 2020

gvanrossum commented Dec 24, 2020

bedevere-bot commented Dec 24, 2020

Fidget-Spinner commented Dec 24, 2020

Fidget-Spinner commented Dec 24, 2020

Fidget-Spinner commented Dec 8, 2020 •

edited

Loading

gvanrossum commented Dec 23, 2020 •

edited

Loading

Fidget-Spinner commented Dec 24, 2020 •

edited

Loading