GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-43224: Initial implementation of PEP 646 in typing.py by mrahtz · Pull Request #24527 · python/cpython

Conversation

@mrahtz

@mrahtz mrahtz commented Feb 14, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Currently this only supports the Unpack[Ts] form, but once the changes from PEP 637 are merged, *Ts also works with no extra changes.

I don't think this is finished yet - we need more tests, especially for aliases - but this is enough to begin discussion.

Notes:

  • I've renamed _TypeVarLike to _BoundVarianceMixin, since TypeVarTuple doesn't support bounds or variance yet, and it would be confusing if TypeVarTuple wasn't a _TypeVarLike. I've created _TypeVarTypes as a replacement.
  • I've renamed _check_generic to _check_type_parameter_count to make its purpose clearer, and overhauled its body to support TypeVarTuples.
  • In _check_type_parameter_count, I've removed the check on whether cls is a generic a) so that the function only does one thing, and b) because I couldn't see any way for the function to be called on a class which wasn't generic (and none of the tests fail, so shrug).

https://bugs.python.org/issue43224

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mrahtz
Initial implementation of PEP 646 in typing.py
f593448
@mrahtz
mrahtz requested review from gvanrossum and ilevkivskyi as code owners February 14, 2021 17:13
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 14, 2021
@bedevere-bot bedevere-bot added the awaiting review label Feb 14, 2021
@mrahtz mrahtz changed the title Initial implementation of PEP 646 in typing.py bpo-43224: Initial implementation of PEP 646 in typing.py Feb 14, 2021
@blurb-it
📜🤖 Added by blurb_it.
1297690
@Fidget-Spinner

Fidget-Spinner commented Feb 14, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Hi! I only took a brief look (sorry I'll try to do a more thorough review if the PEP is accepted), but here's a possible general checklist of things you may have to do:

  • Add to typing.py (done)
  • Add to PEP585 GenericAlias objects' __parameters__ (unless you don't want to treat TypeVarTuple as a TypeVar in builtin generics, sorry I'm not too clear about the PEP's implementation details). You can see how PEP612 ParamSpec added itself in PR bpo-41559: Implement PEP 612 - Add ParamSpec and Concatenate to typing #23702 (it's in Objects/genericaliasobject.c).
  • Ensure some support in typing.get_args and typing.get_origin. Though I think this depends on the extent that the PEP would like to support runtime correctness.

Please correct me if you think I missed anything.

pradeep90 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pradeep90
pradeep90 suggested changes Feb 14, 2021
View reviewed changes

@pradeep90 pradeep90 left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This looks great. Thanks for working on this, Matthew!

Apart from the suggestions above from @Fidget-Spinner, could you also look at typing_extensions.py? I saw a few uses of isinstance(..., TypeVar). Would be worth checking if they need updating.

https://github.com/python/typing/blob/c7a981aaab7b20b93c911f1e5dd0ab2a8e30fd6c/typing_extensions/src_py3/typing_extensions.py#L1136

Also, could you add tests for Tensor[()] and Tensor (without parameters)?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/typing.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Feb 14, 2021
@mrahtz mrahtz mentioned this pull request Feb 20, 2021
Allow variadic generics python/typing#193
Closed
@gvanrossum

gvanrossum commented Mar 1, 2021

Copy link
Copy Markdown
Member

I'm catching up on old items. Is this ready to merge from your POV?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Mar 1, 2021

Copy link
Copy Markdown
Member

I'm catching up on old items. Is this ready to merge from your POV?

Never mind, I didn't read this, I thought it was another PEP update. As an implementation update it will have to wait (but the proof-of-concept implementation is very much appreciated).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Fidget-Spinner

Fidget-Spinner commented Mar 1, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I'm catching up on old items. Is this ready to merge from your POV?

Thanks for doing this. I don't think PEP 646 is accepted by the SC yet so no.

Edit: woops messages posted at the same time and crossed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mrahtz

mrahtz commented Mar 2, 2021

Copy link
Copy Markdown
Contributor Author

Sorry, yes, should have said - just a proof of concept for now, posted here so it would get some initial feedback from the right people.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum
gvanrossum marked this pull request as draft March 2, 2021 16:50
mrahtz added 3 commits March 2, 2021 21:44
@mrahtz
Fix a couple things in PEP 646 implementation
3ba8a47
@mrahtz
PEP 646: Add test for __args__ and __parameters__ of Tuple and classes
d34008f
@mrahtz
Start of work on tuple[*Ts]
69fb16a
@gvanrossum gvanrossum mentioned this pull request Mar 31, 2021
Submission for SC consideration: PEP 646 -- Variadic Generics python/steering-council#59
Closed
@github-actions

github-actions Bot commented Apr 2, 2021

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 2, 2021
@mrahtz mrahtz closed this Jul 11, 2021
@mrahtz
mrahtz deleted the pep646 branch July 11, 2021 11:11
@mrahtz mrahtz mentioned this pull request Dec 2, 2021
Implement PEP 646 in typing_extensions python/typing_extensions#28
Closed
@mrahtz

mrahtz commented Dec 8, 2021

Copy link
Copy Markdown
Contributor Author

This PR is closed, but @Fidget-Spinner, I wanted to ask your advice on something.

After a lot of churn on the PEP itself, I'm restarting work on the CPython implementation. I was going to tackle adding support to genericaliasobject.c, but I happened to notice that some of the work in #23702 you referenced was backed out in 859577c. Before going into this, I thought I should ask - is there anything I should be aware of here? Is genericaliasobject.c still the right place to be working in order to make e.g. list[*Ts] work properly (e.g. adding the corresponding TypeVarTuple to list[*Ts].__parameters__)?

Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Fidget-Spinner

Fidget-Spinner commented Dec 8, 2021

Copy link
Copy Markdown
Member

@mrahtz Sorry, I've lost track of PEP 646 after its many revisions, so I'm not sure what the [*Ts] evaluates to now (does it just evaluate to list[TypeVarTuple[...]] ?).

To clarify, if you want a base implementation for type checkers without the runtime stuff, genericaliasobject.c probably needs no modification, it supports completely arbitrary subscriptions like list[1]. If you want the runtime stuff, you can implement it in either C or Python depending on your needs:

  1. Python way: TypeVarTuple[].__parameters__ should contain itself. This is kind of hacky, but the C genericalias objects blindly take any iterable it sees in any object's __parameters__ and append them to its own. Example:
Y = type("Y", (), {})
y = Y()
y.__parameters__ = (1,)
>>> list[y].__parameters__
(1,)
  1. C "legit" way: modify genericaliasobject.c like what you mentioned above to identify TypeVarTuple directly.

PS. I would love to review the C and typing.py code for the implementation. While I'm useless at the grammar and parser, I'm somewhat better at the bytecode interpreter, builtin objects, and typing.py stuff.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pradeep90

pradeep90 commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

@mrahtz Could you explain what you mean by list[*Ts]? That's not a valid type as per our PEP. Is this something needed for generic aliases or for some edge case in the runtime implementation?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mrahtz

mrahtz commented Dec 10, 2021

Copy link
Copy Markdown
Contributor Author

@pradeep90 Oops, sorry, I meant tuple[*Ts].

@Fidget-Spinner Ah, yeah, tuple[*Ts] basically evaluates to tuple[iter(next(Ts))] now. And great to hear there's a Python way of handling this now! I'll have a play around with this, and let you know if I have any questions - thank you for your kind offer to review stuff :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Dec 10, 2021

Copy link
Copy Markdown
Member

(Why are we discussing this in a closed PR? :-)

Anyway, surely you meant tuple[next(iter(Ts))], not the other way around. But, I don't like the possibility of throwing away extra values which that spelling implies. It is actually hard to find a closed expression that explains what tuple[*Ts] means, since it has a special meaning to type checkers but a different meaning at runtime. I think at runtime it would be equivalent to tuple[tuple(Ts)].

Also, from the runtime's POV tuple is not special, and we need to assign a meaning to X[*Y] for any X and Y. IMO it should always mean X[tuple(Y)]. We're now getting in dangerous territory though, since when keyword args for subscripts were being discussed, python-ideas nearly exploded over the question of what argument X.__getitem__ should see when Y is exactly one item long: does this call X.__getitem__(tuple(Y)) or X.__getitem__(tuple(Y)[0])? (This is relevant because X[1] does not pass a tuple, but X[1, 2] passes the tuple (1, 2). This in turn is a 20-plus-year-old API design issue for __getitem__ that we can't fix.)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mrahtz mrahtz mentioned this pull request Jan 4, 2022
bpo-43224: Draft implementation of PEP 646 #30398
Closed
1 of 2 tasks
@mrahtz mrahtz mannequin mentioned this pull request Apr 11, 2022
Add support for PEP 646 #87390
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum Awaiting requested review from gvanrossum
@ilevkivskyi ilevkivskyi Awaiting requested review from ilevkivskyi
@JelleZijlstra JelleZijlstra Awaiting requested review from JelleZijlstra JelleZijlstra will be requested when the pull request is marked ready for review JelleZijlstra is a code owner
@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood AlexWaygood will be requested when the pull request is marked ready for review AlexWaygood is a code owner
1 more reviewer
@pradeep90 pradeep90 pradeep90 requested changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@mrahtz @Fidget-Spinner @gvanrossum @pradeep90 @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.