Repository navigation
Structural Pattern Matching (PEP 634) #86294
Description
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:
assignee = 'https://github.com/brandtbucher'
closed_at = <Date 2021-04-21.03:24:32.247>
created_at = <Date 2020-10-23.15:22:20.943>
labels = ['interpreter-core', 'type-feature', '3.10']
title = 'Structural Pattern Matching (PEP 634)'
updated_at = <Date 2021-07-27.04:27:58.725>
user = 'https://github.com/brandtbucher'bugs.python.org fields:
activity = <Date 2021-07-27.04:27:58.725>
actor = 'andrei.avk'
assignee = 'brandtbucher'
closed = True
closed_date = <Date 2021-04-21.03:24:32.247>
closer = 'brandtbucher'
components = ['Interpreter Core']
creation = <Date 2020-10-23.15:22:20.943>
creator = 'brandtbucher'
dependencies = []
files = []
hgrepos = []
issue_num = 42128
keywords = ['patch']
message_count = 56.0
messages = ['379442', '379443', '379831', '379833', '379834', '386673', '386702', '386712', '386717', '386720', '386727', '386748', '387362', '387364', '387365', '387369', '387371', '387381', '387384', '387385', '387386', '387389', '387390', '387392', '387409', '387410', '387753', '387769', '387773', '387801', '387815', '387817', '387818', '387819', '387827', '387836', '387968', '388040', '388044', '388046', '388066', '388554', '388555', '388567', '388580', '388583', '388584', '388772', '388775', '388784', '388785', '388790', '388792', '388979', '390270', '391472']
nosy_count = 14.0
nosy_names = ['gvanrossum', 'rhettinger', 'willingc', 'Daniel Moisset', 'lys.nikolaou', 'pablogsal', 'xtreak', 'brandtbucher', 'BTaskaya', 'gousaiyang', 'kj', 'andrei.avk', 'laurenjl', 'freundTech']
pr_nums = ['22917', '24588', '24664', '24667', '24673', '24732', '24733', '25185', '25203', '25642', '27384']
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue42128'
versions = ['Python 3.10']
Activity
brandtbucher commentedon Oct 23, 2020
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.
brandtbucher commentedon Oct 23, 2020
Sorry, just resolving some changes with master. Are you parser people finished breaking my grammar yet? Sheesh. ;)
brandtbucher commentedon Oct 28, 2020
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:
Parts of PEP-634 and PEP-635 can be worked into the language reference.
Guido's overview (the appendix of PEP-636) can probably just be dropped verbatim into the What's New for 3.10.
The rest of PEP-636 can just be lifted into the docs as a proper tutorial and linked to from the What's New.
Also, we need to document the stdlib changes (ast, collections, dataclasses, keyword, ...). The dis module docs are already part of the implementation.
gvanrossum commentedon Oct 28, 2020
If you feel up to it, you might see if you could open a new, separate
(draft) PR that updates all those docs. (But you could also wait and see if
someone volunteers. There are some good doc writers active ATM.)
brandtbucher commentedon Oct 28, 2020
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.
gvanrossum commentedon Feb 9, 2021
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).
Fidget-Spinner commentedon Feb 9, 2021
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:
reference/lexical_analysis.rst
reference/compound_stmts.rst
reference/grammar.rst
# this probably needs a summarized version for non-technical users
whatsnew/3.10.rst
If I missed anything, please point them out :).
dmoisset commentedon Feb 9, 2021
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.
Fidget-Spinner commentedon Feb 9, 2021
Hi Daniel, wow thanks for the offer! My GitHub username is Fidget-Spinner.
I'm not sure about the grammar file either.
Oh, thanks for mentioning the tutorial recommendation by the SC, I'll keep that in mind.
lysnikolaou commentedon Feb 9, 2021
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
brandtbucher commentedon Feb 9, 2021
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.
32 remaining items
freundTech commentedon Mar 12, 2021
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.
brandtbucher commentedon Mar 13, 2021
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.
gvanrossum commentedon Mar 13, 2021
This does point out that the ast structure is a liability, and we won't be
able to easily change it, since mypy and IIRC many linters use the ast
module. So I think that maybe we need to think more carefully about what
the *proper* ast structure is, rather than what was most convenient for the
initial implementation. (Brandt, I know you are reluctant to change this,
and I don't blame you, but nevertheless I think we should at least think
about this some more so we're not stuck with a sub-optimal "default"
solution. We've already won the war. Let's not lose the battle. :-)
brandtbucher commentedon Mar 13, 2021
Yup, I agree we should explore this area further. See my comment here on Adrian’s mypy PR:
python/mypy#10191 (comment)
brandtbucher commentedon Mar 15, 2021
@BTaskaya, do you have any interest in helping me iterate on new AST nodes?
isidentical commentedon Mar 15, 2021
Sure!
brandtbucher commentedon Mar 15, 2021
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.
brandtbucher commentedon Mar 15, 2021
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.
gvanrossum commentedon Mar 15, 2021
Yeah, feel free to close this issue.
brandtbucher commentedon Mar 16, 2021
Actually, I didn't see that there are still 2 open linked PRs. I'll wait until those are merged.
pablogsal commentedon Mar 18, 2021
New changeset 08fb8ac by Pablo Galindo in branch 'master':
bpo-42128: Add 'missing :' syntax error message to match statements (GH-24733)
08fb8ac
brandtbucher commentedon Apr 6, 2021
New changeset f84d5a1 by Brandt Bucher in branch 'master':
bpo-42128: __match_args__ can't be a list anymore (GH-25203)
f84d5a1
isidentical commentedon Apr 20, 2021
I've moved the AST validator to its own separate issue so feel free to close this Brandt!