-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Structural Pattern Matching (PEP 634) #86294
Comments
PEP-634 has not yet been accepted, but we'd like to hit the ground running and get this into alphas as soon as it (hopefully) is. Several people have volunteered to review the implementation, since it's so huge. Other reviews are very welcome, if anybody has a bit of time to pitch in. This touches tons of stuff: the parser, the compiler, the VM, the builtins, the stdlib, the tests... I'd like as many eyeballs as possible! I'll have a draft PR up against master in a few minutes. Let's try to keep all of the review comments there. |
Sorry, just resolving some changes with master. Are you parser people finished breaking my grammar yet? Sheesh. ;) |
Thinking ahead... A lot of work has gone into writing these PEPs... we should see how much we can easily convert into actual docs. It seems to me:
Also, we need to document the stdlib changes (ast, collections, dataclasses, keyword, ...). The dis module docs are already part of the implementation. |
If you feel up to it, you might see if you could open a new, separate |
I'll wait till the SC makes a ruling, then send a message to our docs list (I think we have one)? I'm fine coordinating/reviewing that, or making PRs myself if nobody else steps up. |
The PEP has been accepted. I propose that we use separate PRs for docs (and incremental improvements to the code) but tie them all together using the same issue number (this will require adding "skip news" to later PRs). |
Guido and Brandt, may I take a stab at the docs for this? I'll probably base most of it off PEP-636's tutorial. Relevant parts of the docs that I've identified: # this probably needs a summarized version for non-technical users If I missed anything, please point them out :). |
Hey Ken, I was about to take on this myself, but I wouldn't mind a bit of help. Your list seems ok, I would add to it a section about PM in the python tutorial (which was explicitly mentioned in the SC acceptance notice). Regarding the grammar, I believe that one is autogenerated from the parser, so it's likely that no changes are required there. Correct me if I'm wrong here. I'll set up a branch in github and add you as a collaborator so we can prepare a PR. I'll base the branch of Brandt's implementation PR. If you give me your github id I can add you as collaborator so we can work there. |
Hi Daniel, wow thanks for the offer! My GitHub username is Fidget-Spinner. |
The grammar is indeed auto-generated by reading the actual Grammar file, python.gram. Here's the lexer Pablo had developed for doing the syntax-highlighting. https://github.com/python/cpython/blob/master/Doc/tools/extensions/peg_highlight.py |
Also, see my msg379831 above. We can't entirely rely on the PEPs, of course, but I think we could still get some decent reuse out of them. BTW, has the new docs WG started up yet? I keep hearing about it every once in a while, but I'm not sure if it's fully-formed yet. Could be good to loop them in for review and such, if so. +nosy Carol since I'm pretty sure she's involved with that. |
@ken: I've invited you to the repo. For tracking the WIP, https://github.com/dmoisset/cpython/tree/patma-docs Do you want to see if you can base the reference/compound_stmts.rst changes in PEP-634? I'll try to fit a variant of the Appendix A of PEP-636 into the tutorial and make something up for the reference/lexical_analysis.rst (which needs a section of soft keywords which are not in the PEP trilogy. Perhaps I can rescue something from PEP-622) |
To the folks working on docs: Does it seem realistic to have something ready by the next alpha (March 1st)? I'd like to at least have a What's New entry and a rough draft tutorial by then, since we'll probably (hopefully?) have a bunch of users jumping on that release. I can help if needed, since it looks like I'm mostly waiting on more PR review through the weekend. Or I can stay out of the way. :) |
I don't know who's really in charge of the docs. I suppose the PEP authors Would people be okay if I added the tutorial from Appendix A of PEP-636 to Okay, I'll just add the PR and we can take it from there. |
Yes please! It's not a huge deal, but I vote that we either drop or rework the "http_error" examples. I think it gives people a very wrong first impression that makes the rest of the behavior quite surprising. Can it be changed to build off of something familiar, like unpacking? I like 636's approach much more. |
So 636 Appendix A is identical to the tutorial in the README of the patma repo. It uses http errors for the very first example only, which introduces literal matching. The main text of 636 is too long for what's new IMO, and also a bit unfinished. It is meant as a gentle intro. The Appendix is meant as an intro to pattern matching for experienced Python users (the kind of people for whom the what's new series of documents is written). This tutorial takes the user from the simplest forms of patterns (literals) via more complex ones (tuples, classes) to advanced concepts like nesting patterns, and then just lists a whole bunch of other features. I started with http errors because 404 must be the world's famous number by now, well before pi or even 0 and 1. :-) See for yourself: #24588 |
I understand. I would just like to see something that won't give new Python pattern-matching users (read: everybody) the very painful first impression that this is a switch. Can we rework it like: match input().split(): Or something? (Pardon the example, I don't write many tutorials...) I've seen too many knee-jerk reactions over the past weeks along the lines of "the new switch feature can't handle named constants!". My hope is something like the above might provide a more accurate, informative intro. |
Let's continue this at #24588 |
Here are some that I found interesting: https://twitter.com/jakevdp/status/1359870794877132810 https://twitter.com/brandon_rhodes/status/1360226108399099909 |
Okay, I see. Clearly we should have kept the "DWIM" option, aka "If it starts with a capital letter, it's a value pattern." :-) |
But seriously, nothing good can come from social media. It's what drove me to retire in 2018. Just ignore it please. |
@brandt Bucher <brandtbucher@gmail.com> @guido van Rossum On Fri, Feb 19, 2021 at 8:27 PM Guido van Rossum <report@bugs.python.org>
|
Carol, the most urgent thing we have going is to come up with text for |
Absolutely, I can help do that. On Fri, Feb 19, 2021 at 9:14 PM Guido van Rossum <report@bugs.python.org>
|
Fyi, I'm looking into the documentation. I think I can have something basic for the middle of this week, so it should be ok for March 1st. |
Note: I will NOT write something for "What's New in 3.10" seeing that's in progress |
Thanks @XTreak, I'll make some changes in these sections too. The docs are coming along well, there's still some refinement to do in the compound statements section (it's there, but looks to big), I'll submit a draft PR during the weekend so interested people can review |
Folks, The What's New PR is open now. I've tried to focus more on the data type/shape examples over literal example. |
@BTaskaya, do you think you'll have time to open a PR with your AST validator this weekend? It looks good to me (assuming tests pass). Also, we should add the AST docs to our documentation to-do list (should be just adding entries for ast.Match, ast.MatchAs, ast.MatchOr, and ast.match_case). I'll try to find time to review the open doc PRs today. |
Unfortunately not. I believe it still lacks tests for invalid cases, but other than that should work. If you'd like to take it on feel free, if not I'll create a PR next weekend with tests (probably after release, though I believe it is not a blocker as is). |
I can pick up the AST docs PR |
Thanks Pablo!
No problem, I'm pretty busy too. Next weekend is fine. |
I'm currently working on some performance benchmarks for PEP-634: https://github.com/brandtbucher/patmaperformance Hopefully they will help inform future improvements. I already have benchmarks for class patterns and mapping patterns, and am still searching for a good problem that really stresses all of the different variations of sequence patterns. Maybe a generalized Tower of Hanoi or something? |
Should __match_args__ be added to Objects/structseq.c ? |
Yeah, probably. I'm not too familiar with the design of those objects... would it make more sense to have the implementation be a single descriptor shared by all derived types, or should we precompute the tuple of strings when each new type is defined? I'm also not entirely sure how "unnamed fields" work, or what they're used for. Presumably we'd want to end the __match_args__ tuple upon hitting one of these? |
Opened #24732
AFAIK, they should be just excluded from __match_args__ |
For the last few days I've been working with pattern matching and it's ast for a bit, while trying to add support for it to mypy. During this I noticed an inconsistency in the ast: ast.MatchAs has an attribute name which is of type identifier (in C) and type str (in python). This seams really inconsistent with the rest of the ast, where identifiers are always wrapped in a ast.Name object. The only other exception to this is ast.Attribute. Judging from Grammar/python.gram this seems deliberate: as_pattern[expr_ty]: Could someone shed some light on why MatchAs directly references an identifier instead of an ast.Name? |
import ... as <identifier>
from ... import ... as <identifier>
try:
...
except ... as <identifier>:
... global <identifier> All <identifier>s above are represented as strings. If <identifier> is guaranteed to be a single name, then it makes sense to just use an identifier instead of wrapping it with Name(). The reason that other stuff like "with ... as <expr>" uses <expr> instead of <identifier> is that you can use extended assignment targets (such as unpacking a tuple) over there. This rule applies to all other stuff, except for NamedExprs. Which I believe it is because the restriction might lift in the future, but now they are limited to only keep Name()s. I don't personally know why Brandt choosed to use identifier, though I'm a +1 on the current approach. |
Thanks for the response. Looks like I overlooked the imports, global, nonlocal, ... because I only searched for usages of identifier, but they use lists of identifiers. In that case I agree that it isn't inconsistent. |
Not a much thought went into that decision, honestly. The main reason I chose an identifier there was because "as" always stores a simple name, so allowing it to potentially be other contexts or expressions (which are illegal) just seemed to create complexity. I saw we used simple identifiers in many similar places and just went with it. (As a related note, if mapping patterns had their own AST nodes, I would probably choose to represent "**rest" as an optional identifier, rather than a keyless value or optional Name.) If it causes problems for clients of the AST, it's easy to change. But I sort of like it a bit better how it is now. |
This does point out that the ast structure is a liability, and we won't be |
Yup, I agree we should explore this area further. See my comment here on Adrian’s mypy PR: |
@BTaskaya, do you have any interest in helping me iterate on new AST nodes? |
Sure! |
Cool. Today or tomorrow I'll create an issue in Guido's patma repo and we can move our discussion there. I'll also create a branch and add you to my fork. |
Also, given that pattern matching is now shipped and documented, perhaps this issue should be closed? At this point I think dedicated issues are probably better for any new bugs/enhancements/etc. |
Yeah, feel free to close this issue. |
Actually, I didn't see that there are still 2 open linked PRs. I'll wait until those are merged. |
I've moved the AST validator to its own separate issue so feel free to close this Brandt! |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: