PEP 646: Add section on grammar changes #2039

mrahtz · 2021-07-20T15:03:01Z

pep-0646.rst

JelleZijlstra

Thanks, this is clear to me now!

This is just my opinion and I'm not going to be the one making decisions here, but to me change (1) (* in subscripts) feels stronger than change (2) (* in *args annotations). (1) is a fairly natural extension of the grammar that could be independently useful. (2) is a special case for one very specific situation, so it is more ad hoc.

gvanrossum

(I'm running out of time to review the second half of the diff, here's what I have for the first half.)

pep-0646.rst

gvanrossum · 2021-07-20T21:44:45Z

pep-0646.rst

+    class TypeVarTuple:
+        def __init__(self, name):
+            self._unpacked = _UnpackedTypeVarTuple(name) 
+        def __iter__(self):
+            yield self._unpacked


Maybe note that this is just a trick to make it print the right thing when its repr() is called. It is not helpful for static checking (static checkers don't call iter(), they inspect the AST), nor for dynamic checking (you'd have to recognize _UnpackedTypeVarTuple -- whose existence you hypothesize but never really explain).

Ideally I think dynamic checkers would be able to recognise _UnpackedTypeVarTuple. I've added what I imagine _UnpackedTypeVarTuple to look like (and removed the underscore, since in fact it's meant to be visible to things outside typing.py) - is this enough?

gvanrossum · 2021-07-20T22:34:27Z

pep-0646.rst

+star-unpacking in other contexts, ``_UnpackedTypeVarTuple`` would always be received
+by ``__class_getitem__`` as part of a tuple:


To describe the semantics of this grammar, we need neither _UnpackedTypeVarTuple nor __class_getitem__. Rather, we need to describe what the instance method __getitem__ will receive when the argument has various forms involving *.

Fortunately it seems to be that in all cases where the substitution is valid, x[..., *a, ...] produces the same as x[(..., *a, ...)], with the one edge case that x[*a] becomes x[(*a,)]. Of course if the dots involve a slice, the substitution is not valid, but we can then replace slices with calls to slice(...) to make things valid, for example x[1:2, *a] will become x[(slice(1, 2), *a)]. And then we can just defer to the existing semantics for *a and for x[y].

We probably will need to defend why we even allow x[1:2, *a], since we don't need it for variadic types. Our case is somewhat weak here (and we can state that we'd be fine if the SC were to strike this), but to me it seems that it's the simplest grammar change. After all that's also why we chose to allow e.g. x[*a, *b].

Mm, great points. I've rewritten this section to explain these points (incorporating your wording, which I thought was very clear). What do you think?

gvanrossum · 2021-07-20T22:38:55Z

pep-0646.rst

+    idxs_to_select = (1, 2, 3)
+    array[0, *idxs_to_select, -1]


Yes it would, but PEP 637 was rejected because there weren't enough perceived benefits. And honestly the example doesn't make a lot of sense to me. (Is it equivalent to array[0, 1, 2, 3, -1]?)

I've rewritten this section to make it clearer this is a side effect rather than something we necessarily endorse, and to clarify what this example is equivalent to - better?

pep-0646.rst

gvanrossum

Here's the rest. Not too much new stuff.

pep-0646.rst

gvanrossum · 2021-07-21T18:31:10Z

pep-0646.rst

+    >>> Ts = TypeVarTuple('Ts')
+    >>> def foo(*args: *Ts): pass  # Equivalent to `*args: tuple(Ts)`
+    >>> foo.__annotations__
+    {'args': (UnpackedTs,)}  # Where UnpackedTs is Ts._unpacked


You don't define or explain UnpackedTs. How does it relate to _UnpackedTypeVarTuple? Is this literally what it will print?

Have clarified this - better?

gvanrossum · 2021-07-21T18:38:31Z

pep-0646.rst

+
+    star_annotation[expr_ty]: ':' a=star_expression { _PyAST_Tuple(_PyPegen_singleton_seq(p, a), Load, EXTRA) }
+
+This accomplishes the desired outcome (making ``*args: *Ts`` not be a syntax error)


I think you also need to explain the semantics when the thing after the star is not a TypeVarTuple. If I have

class X: ... x = X() def f(*args: *x): ... print(f.__annotations__)

What will it print? What calls to methods of X are made?

Have tried to clarify this; better now?

gvanrossum · 2021-07-21T18:42:21Z

pep-0646.rst

+1. **Support change 1 but not change 2**. Starred expressions within indexes are
+   more important to us than the ability to annotation ``*args``.


…*args: *Ts

mrahtz · 2021-07-26T10:03:08Z

Thanks for the detailed comments, Guido - these are very helpful!

gvanrossum · 2021-07-26T13:26:13Z

I’m on vacation for two weeks, will be sluggish to review again.

mrahtz · 2021-07-27T03:40:09Z

Sure - enjoy!

gvanrossum · 2021-08-16T15:53:03Z

Ugh, I lost track of this. Will review later today (after the Tensor Typing meeting).

gvanrossum

Basically LGTM -- I only have very small nits left!

Thanks for your patience.

gvanrossum · 2021-08-17T20:12:59Z

pep-0646.rst

+(Note that ``from future import annotations`` does not help here, since no annotations
+are involved.)


This remark is a bit mysterious. Why would a reader make the mistake of thinking that would help? What might they think it helps for?

Also, it's __annotations__.

from __future__ import annotations

I added this based on one of your earlier comments:

The first grammar change enables use of star expressions in index operations (that is,
within square brackets), necessary to support star-unpacking of TypeVarTuples.

I think this should summarize the reason for this change here. The first Summary example shows it well:

class Array(Generic[DType, *Shape]):

That this is the very first example shows how fundamental this is. (And note that "from future import annotations" doesn't help in this position, since it is not technically an annotation.)

I thought you were gesturing towards the fact that with future annotations, new syntax is unnecessary for being able to do whatever manner of exotic things in x: Tensor[A, B, C, D] because of annotations not being evaluated at runtime - so a naive reader might think the same was true for everything else suggested in the PEP.

Anyway, I also thought it was a bit unclear in the end, so I'm happy to remove it.

gvanrossum · 2021-08-17T20:25:20Z

pep-0646.rst

+for ``args``. In other words, ``*args: *foo`` is equivalent to
+``*args: tuple(foo)``.


I'd add "at runtime" somewhere in this last sentence, to make it absolutely clear that this is not an equivalence for static type checkers.

gvanrossum · 2021-08-17T20:26:53Z

pep-0646.rst

+    >>> l = [1, 2, 3]
+    >>> def bar(*args: *l): pass  # Equivalent to `*args: tuple(l)`


I'd use a different variable name than l, which is easily confused with either 1 or I depending on the font.

gvanrossum · 2021-08-17T20:28:09Z

pep-0646.rst

@@ -867,6 +867,7 @@ After:

    star_etc:
    | '*' param_no_default param_maybe_default* [kwds]
+    # This is the new alternative. Other alternatives are unchanged.


With whole-line comments it's always a bit ambiguous where they point. Maybe just add # New to the end of the new alternative?

mrahtz · 2021-08-18T10:14:06Z

Thanks, Guido!

gvanrossum

Yay! I'll merge this now, it's a very nice update.

Before we go back to the SC, let's make sure we check these boxes:

Have we motivated sufficiently why we prefer the *Ts notation over Unpack[Ts]? (I think it's much more readable.)
Have we collected enough endorsements from potential users? We should probably link to issues or messages (e.g. in the mailing list archives) in a brief section labeled "Endorsements" -- the current SC seems to like such things. (We should also be transparent about the non-endorsement from PyTorch.)

PEP 646: Add section on grammar changes

2e40622

mrahtz requested a review from gvanrossum as a code owner July 20, 2021 15:03

the-knights-who-say-ni added the CLA signed label Jul 20, 2021

JelleZijlstra reviewed Jul 20, 2021

View reviewed changes

pep-0646.rst Outdated Show resolved Hide resolved

pep-0646.rst Outdated Show resolved Hide resolved

mrahtz added 3 commits July 20, 2021 19:59

PEP 646: Clarify how unpacked TypeVarTuples would behave

af4c363

PEP 646: Further clarify behaviour of *args: *Ts

5a9f67d

PEP 646: Simplify explanation of how *args: *Ts behaces

e0bd2a1

JelleZijlstra approved these changes Jul 20, 2021

View reviewed changes

gvanrossum reviewed Jul 20, 2021

View reviewed changes

gvanrossum reviewed Jul 21, 2021

View reviewed changes

mrahtz and others added 4 commits July 26, 2021 09:14

PEP 646: Fix first set of Guido's suggestions

f3bf402

PEP 646: Rewrite description of grammar change semantics to be clearer

53bb98b

PEP 646: Fix Guido's second set of suggestions

c656fab

PEP 646: Clarify which alternative is new in the grammar changes for …

c9236e2

…*args: *Ts

mrahtz mentioned this pull request Jul 26, 2021

Submission for SC consideration: PEP 646 -- Variadic Generics python/steering-council#59

Closed

gvanrossum reviewed Aug 17, 2021

View reviewed changes

PEP 646: Final tweaks suggested by Guido

3502def

gvanrossum approved these changes Aug 18, 2021

View reviewed changes

gvanrossum merged commit d70e05a into python:master Aug 18, 2021

mrahtz deleted the pep646 branch August 22, 2021 18:03

erlend-aasland mentioned this pull request May 13, 2022

pep8/greppable exception messages erlend-aasland/peps#1

Closed

erlend-aasland mentioned this pull request Jun 27, 2022

pep 687/mark as accepted erlend-aasland/peps#2

Closed

		star-unpacking in other contexts, ``_UnpackedTypeVarTuple`` would always be received
		by ``__class_getitem__`` as part of a tuple:


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

		This accomplishes the desired outcome (making ``args: Ts`` not be a syntax error)

		1. Support change 1 but not change 2. Starred expressions within indexes are
		more important to us than the ability to annotation ``*args``.

		(Note that ``from future import annotations`` does not help here, since no annotations
		are involved.)

		for ``args``. In other words, ``args: foo`` is equivalent to
		``*args: tuple(foo)``.

		>>> l = [1, 2, 3]
		>>> def bar(args: l): pass # Equivalent to `*args: tuple(l)`

Uh oh!

PEP 646: Add section on grammar changes #2039

PEP 646: Add section on grammar changes #2039

Uh oh!

Conversation

mrahtz commented Jul 20, 2021

Uh oh!

Uh oh!

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrahtz Jul 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrahtz commented Jul 26, 2021

Uh oh!

gvanrossum commented Jul 26, 2021

Uh oh!

mrahtz commented Jul 27, 2021

Uh oh!

gvanrossum commented Aug 16, 2021

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrahtz Jul 26, 2021 •

edited

Loading