← 返回首页
bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing by Fidget-Spinner · Pull Request #23702 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

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

Merged
gvanrossum merged 27 commits into
python:masterfrom
Fidget-Spinner:pep612
Dec 24, 2020
Merged

bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing#23702
gvanrossum merged 27 commits into
python:masterfrom
Fidget-Spinner:pep612

Conversation

Fidget-Spinner commented Dec 8, 2020
edited
Loading

Copy link
Copy Markdown
Member

Changes:

  • Add ParamSpec and Concatenate to typing.
  • Allow both in Callable
  • Allow ParamSpec in Generic
  • Treat ParamSpec as a TypeVar (add them to __parameters__ ) in:
    • typing generics
    • types.GenericAlias
  • Generics parameterized with ParamSpec and subscripted with lists should cast those lists to tuple:
    • typing.Generic
    • types.GenericAlias
  • Tests for ParamSpec
    • Test specified use cases with typing.Callable.
    • Test specified use cases with collections.abc.Callable.
    • Test user defined generics
    • TypeVar like substitution
  • Tests for Concatenate
  • Loosened Generic and GenericAlias type checks to allow for nearly anything.

Documentation will be another patch.

https://bugs.python.org/issue41559

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

gvanrossum 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

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... :-)

Comment thread Lib/typing.py Outdated Show resolved Hide resolved
Comment thread Lib/typing.py Outdated Show resolved Hide resolved
Comment thread Lib/typing.py Outdated Show resolved Hide resolved
Comment thread Lib/typing.py Outdated Show resolved Hide resolved
Comment thread Lib/typing.py Outdated Show resolved Hide resolved
Comment thread Lib/typing.py Outdated Show resolved Hide resolved
Comment thread Lib/typing.py Outdated Show resolved Hide resolved
Comment thread Lib/typing.py Outdated Show resolved Hide resolved
Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved

Copy link
Copy Markdown
Member Author

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.

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

Copy link
Copy Markdown
Member

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

Fidget-Spinner marked this pull request as ready for review December 16, 2020 09:45
Fidget-Spinner marked this pull request as draft December 17, 2020 15:39
17 hidden items Load more…

Copy link
Copy Markdown
Member

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 commented Dec 23, 2020
edited
Loading

Copy link
Copy Markdown
Member

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

gvanrossum 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

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

Comment thread Lib/_collections_abc.py Outdated Show resolved Hide resolved
Comment thread Lib/_collections_abc.py Show resolved Hide resolved
Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved
Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved
Comment thread Lib/test/test_typing.py Outdated Show resolved Hide resolved
3 hidden conversations Load more…
Comment thread Lib/typing.py Show resolved Hide resolved
Comment thread Lib/_collections_abc.py Show resolved Hide resolved
Comment thread Objects/genericaliasobject.c Outdated Show resolved Hide resolved
Comment thread Objects/genericaliasobject.c Outdated
}
// 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)) {

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Why only do this for ParamSpec?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

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 :).

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

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. :-)

Comment thread Objects/genericaliasobject.c Show resolved Hide resolved

gvanrossum 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

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?

Comment thread Lib/_collections_abc.py Outdated Show resolved Hide resolved

Fidget-Spinner commented Dec 24, 2020
edited
Loading

Copy link
Copy Markdown
Member Author

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.

Comment thread Lib/_collections_abc.py Outdated Show resolved Hide resolved

Copy link
Copy Markdown
Member

I think it’s ready!

Copy link
Copy Markdown
Member

@Fidget-Spinner Let me know if you agree.

Copy link
Copy Markdown
Member Author

@Fidget-Spinner Let me know if you agree.

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

Copy link
Copy Markdown
Member

I figured that was the case. :-)

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

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member Author

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

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Footer

© 2026 GitHub, Inc.