Skip to content
This repository has been archived by the owner on Nov 21, 2022. It is now read-only.

Variadic positional args? #136

Open
gvanrossum opened this issue Jul 16, 2020 · 5 comments
Open

Variadic positional args? #136

gvanrossum opened this issue Jul 16, 2020 · 5 comments

Comments

@gvanrossum
Copy link
Owner

Oscar Benjamin is proposing to allow *rest in class patterns, together with some way in __match_args__ to indicate this is allowed.

See https://mail.python.org/archives/list/python-dev@python.org/thread/V6UC7QEG4WLQY6JBC4MEIK5UGF7X2GSD/

I personally don't think it's urgent to add this to the current PEP but it's an interesting thing to keep in mind for the future (probably together with the __match__ proposal when we eventually get to it.)

@Tobias-Kohn
Copy link
Collaborator

Yes, Oscar Benjamin's use case is interesting and worth keeping in mind when designing the general __match__ protocol. And I concur that this is not something that should go into the current PEP, but be kept in mind for future discussions.

I actually tried to reply to his post, but his latest post indicates that, for whatever reason, he is completely ignoring it and holds on to this misconception of __match_args__ having to reflect the parameter list of __init__.

To give a brief summary of my answer for the initiated: the sympy objects he is referring to have a 'sequential structure' similar to tuples, but with an additional class/type. This is somewhat hidden at first as all the data is stored in a single attribute. The actual structure of an object is correctly represented by the pattern Add( args=(Mul(), Mul()) ), whereas the indented structure would much rather be Add( Mul(), Mul() ) (where Add() could have any number of arguments).

This could be implemented either through support of the sequence protocol, i.e. __len__ and __getitem__—which we excluded deliberately because of various issues. Or we could implement a custom __match__ protocol that could return a sequential representation of an object.

In his first post, Oscar Benjamin also mentioned that performance is critical for his intended use case of pattern matching. I therefore believe that any funny tweaks and trickery along the lines of having *rest in the __match_args__ or so is not really appropriate anyway, as it would clearly introduce a number of additional tests and checks. Not to mention, of course, that it probably won't do to get rid of __match__ because of its complexity, just to reintroduce said comlexity with __match_args__ afterwards.

@viridia
Copy link
Collaborator

viridia commented Jul 16, 2020

Also, given the large number of discrete types he is talking about, using a match statement is probably going to perform poorly, since we don't generate a dispatch table.

@gvanrossum
Copy link
Owner Author

I actually tried to reply to his post, but his latest post indicates that, for whatever reason, he is completely ignoring it and holds on to this misconception of __match_args__ having to reflect the parameter list of __init__.

Maybe part of that is a misconception, but IMO his argument from an ergonomics POV makes sense: even with just two levels of nesting, writing

    case And(args=(Or(args=(A, B)), Or(args=(C, D)))) if C == D:

is rather ugly compared to the ideal

    case And(Or(A, B), Or(C, D)) if B == D:

and even the form with doubled parentheses is fairly ugly:

    case And((Or((A, B)), Or((C, D)))) if B == D:

@viridia

Also, given the large number of discrete types he is talking about, using a match statement is probably going to perform poorly, since we don't generate a dispatch table.

I'm not sure that follows. I doubt that he's proposing to write match statements with hundreds of cases.

@gvanrossum
Copy link
Owner Author

Oh, I just re-read his post and noticed this:

[...] Note that performance is a concern: just dispatching on isinstance() has a measurable overhead in sympy code which is almost always CPU-bound.

That does sound like it might not be easy to improve upon.

Although he immediately continues

The main problem though is with variadic positional arguments. [...]

@Tobias-Kohn
Copy link
Collaborator

Oh, don't get me wrong: I think it is highly valid point he has brought up and something we certainly have to consider in the long run. I much rather have my doubts concerning the proposed solution, or even the location of the actual problem.

Even though we more or less started with that idea, I think that closely reflecting the constructor arguments in the patterns is something of a dead end that does not move us much forward. Sure, this works with algebraic data types in functional languages, but objects in Python are quite a different beast. Hence, I do not really see much merit in the idea of including a starred version in __match_args__, e.g. __match_args__ = ('head', '*tail'). The star would obviously have to be encoded as part of the string representing the name, which means that we are encoding part of the structure in something that needs to be parsed for each invocation.

I understand the idea behind it in that __match_args__ could be seen as something like a parameter list. But if we really followed that logic, we would have to come up with something more powerful than a tuple of strings. In fact, we might end up with a structure holding information similarly to code objects, having fiels such as flags, posonly_args, kwargs, etc. This seems like an awful lot of complexity for just a little gain.

Starting from the tuple idea, we could also say that __match_args__ could contains integers to mean that a specific "positional argument" should be compared not against a named attribute, but rather __getitem__(n). For, e.g., a tuple with three elements, we'd have something like __match_args__ = (0, 1, 2). This would be a step along the lines of what you proposed in your response with the __0__, __1__, etc. attributes. But as the solution above, it would add quite a bit of complexity for not really that much gain.

In the end, the most sensible course of action IMHO is to take this example as a corner stone for the future design of __match__, where we can fully address this situation, but rather leave the PEP as is, without any short-time patches. And with that, I have written a lot of text to basically say: I concur with you: 😉

I personally don't think it's urgent to add this to the current PEP but it's an interesting thing to keep in mind for the future (probably together with the match proposal when we eventually get to it.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants