Implement PEP 646 in typing_extensions

@mrahtz

PEP 646 has been accepted (subject to some confusion about an edge case that's still being discussed), and we should have an implementation of it in typing-extensions.

@mrahtz @pradeep90 is there already a runtime implementation of PEP 646 somewhere? If not, I'd be happy to work on getting the right bits into typing-extensions and typing.

Thanks, Jelle. We were working on a runtime implementation in https://github.com/python/cpython/pull/24527/files, but we could improve the current PR (I'd added some comments there). We'd also need to support `Unpack[Tuple[<foo>]]` as per the latest change. If you're still interested in adding those, I'm happy to review. Otherwise, I suppose Matthew or I will probably complete that PR over the coming month. (We will also need to land the `*Ts` syntax changes, but Matthew already has a fork with the implementation <https://github.com/mrahtz/cpython/tree/pep637%2B646>)

…

On Thu, Nov 18, 2021 at 3:13 PM Shantanu ***@***.***> wrote: I assume they use https://github.com/facebook/pyre-check/blob/main/pyre_extensions/__init__.py — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <python/typing_extensions#28>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6L62VEXCH2YIIFKYXURDUMWCBLANCNFSM5IKZXLDQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- S Pradeep Kumar

(+1 to what Pradeep said.)

Thanks! I agree with many of Pradeep's comments on that PR; it seems to add too many runtime checks. I'm happy to pick it back up and submit a new version to CPython and typing-extensions.

Sounds good, Jelle, thanks!

…

On Fri, Nov 19, 2021 at 5:14 AM Jelle Zijlstra ***@***.***> wrote: Thanks! I agree with many of Pradeep's comments on that PR; it seems to add too many runtime checks. I'm happy to pick it back up and submit a new version to CPython and typing-extensions. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <python/typing_extensions#28>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6L674QEYBTRZAOGLXVKDUMZESBANCNFSM5IKZXLDQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- S Pradeep Kumar

That's very kind of you Jelle - thank you!

How much of the CPython work were you intending to do - just the work in typing.py, or also e.g. the non-grammar changes necessary to make list[*Ts] (rather than List[*Ts]) work?

Full disclosure is that my main motivation is to be able to (ab)use typing_extensions.Unpack for precise *args/**kwargs typing, so I want it to exist :). Also, it seems like acceptance of the PEP has been put on hold, so we should probably wait with CPython changes. But I'm happy to help get other parts of the implementation finished.

So my plan would be:

Get TypeVarTuple and Unpack into typing_extensions
Get them into CPython too, together with docs
Incorporate any changes from the CPython review process into typing_extensions
Implement anything else that needs doing. I'm not sure exactly what that may include, but we should at least test cases like list[*Ts] that you mention.

You already have an implementation for the grammar changes, right?

@JelleZijlstra

@JelleZijlstra looks like you're aiming to get a typing_extensions implementation before a typing one. Is that much easier that just doing typing right off the bat? Otherwise I wonder if we should just aim for typing right off the bat

Context on why I care: I'm ready to start trying to get a proof-of-concept for callable syntax, but I realized a blocker is that I'm going to need a way to represent TypeVarTuple and Unpack in cpython itself. So I need the typing code in a branch for my work anyway.

It seems like there may be no harm in putting up a PR on typing and only changing typing_extensions if it bogs down in code review

typing-extensions is harder, not easier, because it needs to deal with typing.py in older Python versions. I want to put it in typing-extensions first for two reasons: the PEP isn't accepted yet, so it doesn't make sense to put it in typing.py; and I'd like to be able to use the new features before Python 3.11 comes out.

@pradeep90

I see - I was just chatting with @pradeep90 and he said the PEP was just accepted (looks like not on the PEP page yet), which is why I was wondering about starting on typing right away, since it partially blocks my work.

If it helps I could work on a PR based on @mrahtz's old code. I care more about typing than the syntax part of the implementation for my proof-of-concept, although it might be useful to work off of a branch with both since I need a similar unpack syntax

@stroxler

Sorry for the slow reply - this thread dropped off my radar.

Re status of the PEP - it was accepted, but then we realised there's one edge case yet to fix, and the Steering Council have said they want to review those changes before giving it the final stamp of approval. So it seems pretty likely that the final version will be accepted, but we're not quite there yet.

You already have an implementation for the grammar changes, right?

Yup, in https://github.com/mrahtz/cpython/tree/pep646-grammar.

@stroxler, if you're wanting something in typing itself soon, I would propose the following breakdown of the work:

@JelleZijlstra implements support in typing_extensions
@stroxler implements support in typing (bpo-43224: Initial implementation of PEP 646 in typing.py cpython#24527 might be useful as a starting point, if you haven't seen it already)
I'll continue the work on CPython itself (adding suppose for list[*Ts] and the other things noted in that PR above)

Not sure whether that makes sense though given the work that @JelleZijlstra has already done - @JelleZijlstra, what do you think?

@mrahtz

@mrahtz I realized I'm probably not hard-blocked on typing.py per se

Instead, what I'm going to need is to mimic the grammar changes inside a case of my callable type. But I also would like to check in about whether I could try an alternative implementation of the same grammar.

After reading all of the docs on the parser and compiler, plus the implementation of PEP 604 using types.Union I am pretty convinced that we should add a types.UnpackType and make star-as-unpack in *args: *Ts resolve to this type.

This will allow us to get a Starred expression in the Ast directly, rather than a Tuple singleton containing a Starred expression, when we have a *args: *Ts annotation.

A code snippet to illustrate what I mean:

>>> import ast
>>> print(ast.dump(ast.parse("def f(*args: *Ts): ..."), indent=2))
Module(
  body=[
    FunctionDef(
      name='f',
      args=arguments(
        posonlyargs=[], args=[],
        vararg=arg(
          arg='args',
          annotation=Tuple(   # this tuple is extraneous
            elts=[
              Starred(      # the AST would make more sense if we get a Starred directly here
                value=Name(id='Ts', ctx=Load()),
                ctx=Load())],
            ctx=Load())),
        kwonlyargs=[],  kw_defaults=[],  defaults=[]),
      body=[Expr(value=Constant(value=Ellipsis))], decorator_list=[])],
  type_ignores=[])

I'm most concerned about the AST having an extra layer since that's what most tools will look at, but the enclosing Tuple also affects runtime behavior - we wind up getting not an _UnpackedTypeVarTuple but rather a singleton tuple containing one at runtime. This feels wrong.

Both these behaviors can be changed if we make an UnpackType builtin, analogous to UnionType. I'd kind of like to take a stab at it, because I'm going to need to do this quite a lot for a callable syntax reference implementation anyway.

@stroxler

@stroxler this sounds like you're conflating two different things: what the AST looks like, and what it gets compiled to. The PEP says *args would be equivalent to tuple(args), but your UnpackType builtin might imply adding a new dunder method.

Hopefully it doesn’t have to be IDENTICAL? At runtime I want to be able to tell which one they typed.

@gvanrossum

@gvanrossum that's what the PEP specifies: https://www.python.org/dev/peps/pep-0646/#change-2-args-as-a-typevartuple. *args in an annotation is equivalent to tuple(args).

I guess we could still change that (at the risk of further annoying the SC). As the author of a tool that would consume runtime annotations, I'm not particularly concerned about being able to tell the difference, though. Why do you think that's important?

I think it makes sense to introduce a new AST node type to represent the *Ts node. This is what we do in general.

Something else to consider here: I just realized that in the current implementation, the CPython unparser phase is guaranteed to create an explicit (*Ts,). That means that when using from __future__ import annotations with this feature, the string annotation we wind up with would have the explicit tuple.

It's not clear whether that actually matters (most libraries interpreting types at runtime would presumably wind up evaluating the string anyway) but it could definitely surprise people.

@zsol

I wonder if it might cause major issues for concrete syntax trees like LibCST to have an "invisible" tuple node with no printable characters (I'd have to ask @zsol)

I don't think this is very relevant to the typing discussion, but since I've been summoned: If the currently proposed implementation gets merged, LibCST's CST will diverge from the AST in this case and will probably introduce a new node type, like @gvanrossum suggests above. There's already precedent for divergence like this, but they tend to cause friction for users of the library.

Would we even need a new AST node? ast.Starred already exists and is an expr. We might just need a compiler change to handle ast.Starred and turn it into a tuple in the bytecode.

@JelleZijlstra

I agree with @JelleZijlstra, I'm pretty certain we don't need a new AST node unless we really want one. And I don't think we should add one if we can help it, using a bare Starred is more with the other unpack contexts (splatting into Callable params lists / splatting into slices) where we really do want the existing Starred type and its runtime semantics.

We could probably compile it into either the equivalent of (*foo,)[0] or the equivalent of next(iter(foo)), both of those options would avoid the need for a new builtin type. I don't think we'd want to leave the value wrapped in a tuple at runtime unless we have to - it would be much nicer if it evaluates to the actual unpacked type.

We might run into objections in code review if we do it that way, because we would have to make Starred compile universally into this, and then rely on the fact that preexisting unpack never directly compiles a Starred expression (which is already the case - the compiler crashes if it sees a Starred top-level expression, which is ruled out by the grammar but not by the Ast)

Okay, using a bare Starred node here should work fine.

Alright, I'll start working on the grammar/AST/compiler changes to make this happen.

Ok, so this turned out to be easier than I expected; I've got a first attempt done in mrahtz/cpython@d76039a.

Grammar: instead of

star_annotation[expr_ty]: ':' a=star_expression { _PyAST_Tuple(_PyPegen_singleton_seq(p, a), Load, EXTRA) }`

we just do

star_annotation[expr_ty]: ':' a=star_expression { a }

This is the only change needed compared to what's currently in the PEP.

Compiler: instead of changing the implementation for Starred_kind in the main visit table, we add a special case to the visit function for argument annotations emitting (what I hope should be) the bytecode for [*Ts][0].

The results are:

AST: print(ast.dump(ast.parse('def foo(*args: *Ts): pass'), indent=2)) gives:

Module(
  body=[
    FunctionDef(
      name='foo',
      args=arguments(
        posonlyargs=[],
        args=[],
        vararg=arg(
          arg='args',
          annotation=Starred(
            value=Name(id='Ts', ctx=Load()),
            ctx=Load())),
        kwonlyargs=[],
        kw_defaults=[],
        defaults=[]),
      body=[
        Pass()],
      decorator_list=[])],
  type_ignores=[])

As for __annotations__, the following code:

from typing import TypeVarTuple

Ts = TypeVarTuple('Ts')
def foo(*args: *Ts):
    pass

print(foo.__annotations__)

gives:

{'args': *Ts}

Does that fix everything we needed it to?

@mrahtz

@mrahtz I'm fine with that, but it's different from the behavior specified in the PEP (https://www.python.org/dev/peps/pep-0646/#change-2-args-as-a-typevartuple), which says that *args: *Ts is equivalent to args: tuple(Ts), not *args: tuple(Ts)[0].

I think that fixes all the concerns. Agreed it requires updating the PEP language slightly.

The precise bytecode we generate is an implementation detail, it might be worth asking advice on python-dev. I would guess Ts.__iter__().__next__() is likely both fastest and the most direct indication of how the python-side code for an unpack annotation works: we're piggybacking on iterable semantics to make unpack work, tuples and lists are only incidentally related.

@JelleZijlstra

@JelleZijlstra Indeed, this would require another change to the PEP.

@stroxler Alright, I've also been able to code up a next(iter(Ts)) version in mrahtz/cpython@a0fcdf8.

@pradeep90 @gvanrossum Any opinions on this - [*Ts][0] vs iter(next(Ts))? Guido, who'd be a good person to ask for a preliminary review on the compiler changes?

By the way @stroxler @JelleZijlstra - Pradeep and I were thinking that implementation development could actually get a bit messy with the work being spread throughout three separate places. I've started https://github.com/mrahtz/cpython/tree/pep646 as the canonical source for the implementation - grammar changes, compiler changes, typing.py changes, and other C changes to support list[*Ts] etc. Is there anything from the work you two have done so far that I can cherry-pick into there?

I haven't done any C work so far, only the draft typing-extensions implementation in python/typing#963 (which doesn't work yet in 3.6).

I haven't done any work either. I'm happy to help out if there's anything you'd like to delegate but I didn't want us to have clashing changes.

Navigation Menu

Repository navigation

Implement PEP 646 in typing_extensions #28

10 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Implement PEP 646 in typing_extensions #28

Description

Activity

pradeep90 commented on Nov 19, 2021

mrahtz commented on Nov 19, 2021

JelleZijlstra commented on Nov 19, 2021

pradeep90 commented on Nov 21, 2021

mrahtz commented on Nov 23, 2021

JelleZijlstra commented on Nov 23, 2021

stroxler commented on Nov 30, 2021

JelleZijlstra commented on Nov 30, 2021

stroxler commented on Nov 30, 2021

mrahtz commented on Dec 2, 2021

stroxler commented on Dec 2, 2021

JelleZijlstra commented on Dec 2, 2021

gvanrossum commented on Dec 2, 2021

JelleZijlstra commented on Dec 2, 2021

10 remaining items

gvanrossum commented on Dec 3, 2021

stroxler commented on Dec 3, 2021

zsol commented on Dec 3, 2021

JelleZijlstra commented on Dec 3, 2021

stroxler commented on Dec 3, 2021

gvanrossum commented on Dec 3, 2021

mrahtz commented on Dec 4, 2021

mrahtz commented on Dec 4, 2021

JelleZijlstra commented on Dec 4, 2021

stroxler commented on Dec 5, 2021

mrahtz commented on Dec 8, 2021

JelleZijlstra commented on Dec 8, 2021

stroxler commented on Dec 9, 2021

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions