Could we autogenerate __match_args__ for ast classes?

[dmoisset]

One of the places where pattern matching can shine is operating on ASTs. So it would be really useful to have good support ast classes from the start. And it shouldn't be really hard: all AST classes have a _fields attribute with a tuple of names, so we could just assign __match_args__ to the same value. We could give some good examples with that that aren't dataclasses :)

[dmoisset]

I thought of doing this myself, but it may take me a bit to wrap my head around the Parser/adsl_c.py module that generates the C code for the _ast types.

[gvanrossum]

Definitely a good idea!

I don't recall if there is such a thing now as a class property. If there is, we could add one for __match_args__ to the base AST class that returns cls._fields.

Or you could just find the code that generates _fields and duplicate it for __match_args__. :-)

[brandtbucher]

Yeah, I can go ahead and do this tonight if nobody else gets to it by then.

[thautwarm]

Doing so is awesome, but as a user often working with ast module, I hope we could avoid matching AST types with positional arguments.

I cannot always recall the indices of fields of AST types, except for ast.{Name, Constant}.

The indices and corresponding __match_args__ could also change, e.g., posonlyarg is newly added as the first parameter of ast.arguments. Such thing breaks the existing code if positional sub-patterns are used, but if we only allow keyword sub-patterns, no breakage but compatibility.

Not only for compatibility, I also think that matching most of AST types with keyword sub-patterns helps a lot to the readability.

[pablogsal]

@brandtbucher I have opened a PR against your branch here: https://github.com/brandtbucher/cpython/pull/6/files

[brandtbucher]

This has been included in the implementation. Do we want to PEP this @gvanrossum? It would probably just be augmenting our comments on named tuples and dataclasses.

Depending on timing, I think scouring the standard library for other classes like these that could really benefit from __match_args__ would make a great PyCon sprint.

[pablogsal]

Depending on timing, I think scouring the standard library for other classes like these that could really benefit from __match_args__ would make a great PyCon sprint.

Regarding the proposed implementation and thinking in a future review if the pep is accepted I would recommend to defer big additions to other classes as the implementation is already big enough so the review will be time consuming. Is easier to spot small errors and behaviour mismatch in future, scoped PRs. Also, what should implement the protocol and what should not probably is easier to discuss individually and it will take less contingency points to the PEP in the rare case someone wants to remove (or likely add) something.

[brandtbucher]

Sorry, I should have been clearer. The current implementation is huge, and I have no desire to make it any harder to review, or to include these changes in the PEP (which is also huge).

I meant that if this has already been merged before the sprints come around, it would be nice to attack everything at once and have unified strategies/goals applied to the whole stdlib, with discussion happening in one place. Those changes would then be reviewed individually, of course.

Again, just an abstract idea.

[gvanrossum]

Great job Pablo!

I don't see why we should PEP this. The PEP already promises

In addition, a systematic effort will be put into going through
existing standard library classes and adding __match_args__ where
it looks beneficial.

I don't think we need to add anything more. (Dataclasses and namedtuples are worth being mentioned as they are the closest thing we have to algebraic types.)

Regarding Taine's comment on positional vs. keyword arguments for AST classes, from a quick look at Python.asdl it seems there are plenty of AST node types that have 1-2 arguments and in almost all cases the ordering is just derived from how the elements occur in the source code (e.g., BinOp(left, op, right)). So I think it's fine. Certainly for the more complicated things I'd recommend using keyword args but I think that's just common sense, we don't need to implement some kind of cut-off.

Regarding everything else in the stdlib, I recommend caution. The stdlib is very big. The only sensible way is to tackle the most common classes first. Fortunately the worst that can happen is that we force people to use keyword arguments to patterns where they might have been able to use positional arguments. If people were to file bugs for that I'd say we had already won. :-)