-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PEP 634, PEP 635, PEP 636: Create the structure for the PEP 622 split #1598
Conversation
pep-0634.rst
Outdated
TODO: rewrite the ``patterns`` rule to be easier to follow -- why | ||
isn't it just ``','.value_pattern+ [',']``? Also, ``value_pattern`` | ||
is a confusing name, since it is unrelated to "constant value |
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.
It's this way because you can't have a starred pattern without a comma.
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.
Hm, then I still propose to give it a better name.
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.
Hm. elts_pattern
and elt_pattern
instead of values_pattern
and value_pattern
? items_pattern
is already taken by mappings.
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.
Let me ponder that, I'm not crazy about the abbreviated name. Maybe multi_pattern
? Also, could we define the name open_pattern
to mean multi_pattern | pattern
? It somewhat nicely contrasts with closed_pattern
. (Although then apparently walrus and OR patterns are neither closed nor open. :-)
pep-0634.rst
Outdated
TODO: Do we use "the subject matches the pattern" or "the pattern | ||
matches the subject"? |
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.
Good question. The latter feels right to me, since our match protocol uses "smart" patterns and "dumb" subjects.
How do you feel about reserving the "magical" optimizations like caching and decision trees for The SC seemed responsive to the idea when I brought it up during our meeting. I've thought about it more and think it's a good compromise (and could maybe even open the door to more aggressive optimizations elsewhere). |
Stopping for now. I'll be back at it later. |
pep-0634.rst
Outdated
interpreter may cache this value in a similar manner as described for | ||
constant value patterns. | ||
|
||
TODO: Do we have left-to-right semantics here? |
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.
Yes. Actually, I think everything is left-to-right except walrus patterns (which in my mind is a decent argument for switching to as
syntax).
I'll remove this line.
Hm, I consider |
Also tweak remark about scope
Add a TODO, remove another
- Remove redundant text about exceptions - Side effects becomes a toplevel section - Reflow stdlib section
No... I may have some time for this tomorrow an on the weekend, and to get a first draft of the rest. |
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.
I really like the new text you've added! Just two small wording nits.
pep-0635.rst
Outdated
Python's establish structural paradigm, leading to additional syntactic | ||
rules: | ||
|
||
- *Do no indent case clauses.* |
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.
I've always found the conventional style in "Rejected ideas" sections in PEPs confusing. By tradition, each rejected idea has a heading that says "Do X" or "Do not do X" (followed by text that explains the idea and then argues why it is rejected), where the sense of the heading is what the PEP doesn't do. The confusion arises when you see such a heading out of context, e.g. after searching the document.
My proposed alternative to this style is to make the headings more neutral. In this case I would perhaps write "unindented case clauses", and for the next section I'd write "Putting the expression on a line after match
" (there are linguistic terms for these different forms but I've never learned them :-).
@Tobias-Kohn See my last comments. |
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.
I am going to apply my remaining suggestions.
None of these should be controversial.
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.
@Tobias-Kohn
Excellent progress! Here are some nits.
You may also want to go over past comments. There is no great way to do that in GitHub, but scrolling down and looking over every comment that hasn't been resolved yet usually works. Then mark as resolved every comment that has either been acted upon or no longer requires action.
pep-0635.rst
Outdated
@@ -294,6 +294,12 @@ that a subject might safely fail to match a specific pattern at any point | |||
pattern should avoid side effects wherever possible, including binding | |||
values to attributes or subscripts. | |||
|
|||
A corner stone of pattern matching is the possibility of arbitrarily |
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.
A corner stone of pattern matching is the possibility of arbitrarily | |
A cornerstone of pattern matching is the possibility of arbitrarily |
pep-0635.rst
Outdated
section above). At any level of the nesting, several patterns can be | ||
combined to form alternatives. |
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.
So, hm, what exactly is "combining patterns"? I know nesting: C(P1, P2)
is a class pattern that has two subpatterns P1 and P2 nested inside it. But what's "combining to form alternatives"? Is this just a reference to P1 | P2
? To me that's still a nested pattern -- it's an OR pattern with two subpatterns, P1 and P2. Maybe we can simplify things by dropping the whole "combining" concept altogether?
pep-0635.rst
Outdated
hand, builds a generalized concept of iterable unpacking. Binding values | ||
extracted from a data structure is at the very core of the concept. Explicit | ||
markers for capture patterns would thus betray the objective of the proposed | ||
pattern matching syntax. |
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.
Maybe we could clarify this a bit more by pointing out that we expect capture patterns to be (significantly) more common than constant value patterns? (So if we were to have to mark either captures or values, we'd have to mark values and keep captures "plain".)
pep-0635.rst
Outdated
@@ -475,13 +495,56 @@ patterns impossible. | |||
Group Patterns | |||
~~~~~~~~~~~~~~ | |||
|
|||
Allowing users to explicitly specify the grouping is particularly helpful | |||
in case of alternatives or sequence patterns written as tuples. |
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.
in case of alternatives or sequence patterns written as tuples. | |
in case of OR patterns. |
Dropping sequence patterns because (P1, P2)
is not a group -- it's a (closed) sequence pattern. The parentheses are a syntactic part of the sequence pattern (unlike tuple expressions).
pep-0635.rst
Outdated
Moreover, the sequence pattern only matches a narrow set of possible | ||
subjects, whereas iterable unpacking can be applied to any iterable. |
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.
Hm, "a narrow set of possible subjects" sounds a bit too narrow. How about "a carefully selected set"?
pep-0635.rst
Outdated
|
||
- As in iterable unpacking, we do not distinguish between 'tuple' and | ||
'list' notation. ``[a, b, c]``, ``(a, b, c)`` and ``a, b, c`` are all | ||
equivalent. |
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.
Maybe add something like "While this means we have a redundant notation and checking specifically for lists or tuples requires more effort (e.g. case list([a, b, c])
), we want to match user-defined sequence types as well.
Or say something about mimicking (sequence) unpacking is much as we can.
pep-0635.rst
Outdated
- The sequence pattern does *not* iterate through an iterable subject. All | ||
elements are accessed through subscripting and slicing, and the subject must | ||
be an instance of ``collections.abc.Sequence`` (including, in particular, | ||
lists and tuples, but excluding strings and bytes). |
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.
Maybe also mention that this specifically excludes sets (whose order is randomized, making it hard to imagine use cases for pattern matching that aren't footguns) and dictionaries (where we'd lose the keys, and for which we already have a separate pattern).
pep-0635.rst
Outdated
lists and tuples, but excluding strings and bytes). | ||
|
||
A sequence pattern cannot just iterate through any iterable object. The | ||
consumation of elements from the iteration would have to be undone if the |
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.
consumation of elements from the iteration would have to be undone if the | |
consumption of elements from the iteration would have to be undone if the |
pep-0635.rst
Outdated
cases for strings and bytes seems different enough from that of tuples and | ||
lists to warrant a clear distinction. Strings and bytes are therefore not | ||
matched by a sequence pattern, limiting the sequence pattern to a very | ||
narrow and specific understanding of 'sequence'. |
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.
Maybe mention that this understanding is actually more intuitive? It's a common footgun and we get regular complaints about it, but in other contexts, sequence operations on strings are so common that even Python 4 won't be able to fix this.
=================== | ||
|
||
Pattern matching emerged in the late 1970s in the form of tuple unpacking | ||
and as a means to handle recursive data structures such as linked lists or |
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.
Hm... I tried to come up with a reasonable example, but it came rather short, so I think this may not be such a great idea.
This PR has become unmanageable. :-( I am landing it as-is (with strong notes at the top of each that these drafts are for the authors' convenience) and then see if I can get the pep-622-split branch to become the new starting point. |
Also change PEP 622 status to "Superseded", and mark that as "Superseded-By: 634".
We won't land this until we have a decent amount of content in each of the three new PEPs (but we'll still iterate after that).