Skip to content

[PEP 646] Clarify both the AST and runtime behavior of *args: *foo #2177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 0 commits into from

Conversation

stroxler
Copy link
Contributor

@stroxler stroxler commented Dec 3, 2021

Two changes here:

(1) Fixed an incorrect statement about runtime behavior: *args: *foo does not behave like *args: tuple(foo) but rather *args: tuple(*foo). In the case of the proposed Unpack the *foo will yield a single _UnpackedTypeVarTuple or something similar, and so we'll wind up with that unpacked value inside a singleton tuple.

(2) Clarified the AST behavior as well, which is possibly more important than the runtime behavior since this is what typecheckers, codemod tools, etc will work with: the starred expression is getting auto-wrapped in a tuple, and we can't actually distinguish between a bare starred expression versus that same expression inside of an explicit tuple literal (which is not the same as a tuple function call).

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@stroxler

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@gvanrossum
Copy link
Member

Whoa, we have to be careful that we don’t rewrite this PEP too radically. Were these points discussed before?

I seriously worry that this PEP might become a train wreck of broken process.

@JelleZijlstra
Copy link
Member

Also, this new text is wrong: tuple(*foo) is not equivalent at runtime to (*foo,).

@stroxler
Copy link
Contributor Author

stroxler commented Dec 3, 2021

Oh you're totally right - there's no way that it could ever be equivalent to a tuple call. We should take that out entirely, it's always a tuple literal

@stroxler
Copy link
Contributor Author

stroxler commented Dec 3, 2021

Updated - both the AST and the runtime behavior (which is obviously fully determined by the AST) are equivalent to (*foo,), neither one is ever equivalent to a form involving tuple.

I don't think we're adding anything new here, just clarifying what the text says with an example to make it unambiguous (and correct).

@pradeep90
Copy link
Contributor

Whoa, we have to be careful that we don’t rewrite this PEP too radically. Were these points discussed before?

I seriously worry that this PEP might become a train wreck of broken process.

I agree with Guido :|

Let's first finalize the CPython implementation and then change this text as needed. Let's move any discussions to Matthew's/Jelle's PR (I've reached out to @mrahtz about this). With actual code and tests there, we will have something concrete to refer to. Otherwise, it can get confusing here and cause a lot of unnecessary churn.

@stroxler
Copy link
Contributor Author

stroxler commented Dec 3, 2021

Sounds good.

I just want to make sure that in that discussion everyone is clear that *foo is definitely not equivalent to tuple(foo), in the current proposed implementation or in any other implementation we would make.

@mrahtz
Copy link
Contributor

mrahtz commented Dec 3, 2021

I've weighed in on this in python/typing_extensions#28.

I just want to make sure that in that discussion everyone is clear that *foo is definitely not equivalent to tuple(foo), in the current proposed implementation or in any other implementation we would make.

I'm not sure I understand on this - in the proposed grammar changes as implemented in https://github.com/mrahtz/cpython/tree/pep646-grammar, I'm pretty sure *foo is equivalent to tuple(foo). I've verified this by testing the following using that branch:

Ts = TypeVarTuple('Ts')

def foo(*args: *Ts): pass
def bar(*args: (*Ts,)): pass
def baz(*args: tuple(Ts)): pass

print(foo.__annotations__)
print(bar.__annotations__)
print(baz.__annotations__)

and finding that they do indeed all output:

{'args': (*Ts,)}
{'args': (*Ts,)}
{'args': (*Ts,)}

Maybe I'm missing something though - could you clarify?

@mrahtz
Copy link
Contributor

mrahtz commented Dec 3, 2021

Oh, hmm, I've just realised one way in which *args: *Ts and *args: tuple(Ts) are not exactly the same is in their AST representation. The former produces Tuple[Starred[...]] (full version below in [1]) in the AST, but the latter produces Call(func=Name(id='tuple', ...), ...) (full version below in [2]) in the AST. Is that what you meant?

Test code using https://github.com/mrahtz/cpython/tree/pep646-grammar:

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

[1]:

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

[2]:

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

@stroxler
Copy link
Contributor Author

stroxler commented Dec 4, 2021

Yep, that's what meant - they do happen to evaluate to the same thing but they aren't really equivalent - the current implementation is equivalent to a tuple literal as opposed to a tuple function call.

Since many of the people consuming details of this PEP will be doing static analysis, we should make sure that the final state of the PEP matches the implementation for the AST, which means it's clearer to tie it to (*foo,) rather than tuple(foo)

That said, I think it's a good idea to let my PR stay open until we've decided a final implementation to avoid thrash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants