GeezerVison is showing github.com — bpo-43224: Initial implementation of PEP 646 in typing.py by mrahtz · Pull Request #24527 · python/cpython
Conversation
mrahtz
commented
Feb 14, 2021
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
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.
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
|
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
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
pradeep90
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
I'm catching up on old items. Is this ready to merge from your POV?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
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.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
This PR is stale because it has been open for 30 days with no activity.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
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!
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@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:
- 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,)
- 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.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@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?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@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 :)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
(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.)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
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.
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.
|