Hi Brandt,

On 18/02/2021 5:32 pm, Brandt Bucher wrote:
Thanks for taking the time to work on this, Mark.

Overall, I'm skeptical of this proposal. It seems like it takes a lot of "simple" things 
and makes them quite complex, and takes many "static" things and makes them quite 
dynamic. I feel that it also misrepresents certain aspects of PEP 634.

Here's a more detailed commentary:

Pattern matching will be more usable for complex classes, by allowing classes 
more control over which patterns they match.

I fear this is at the expense of most simple classes, which currently "just 
work" with PEP 634. I'm not convinced that making it easier for very complicated 
classes (like those in `sympy`) to participate at the expense of everyday classes is a 
win.

In my experience, nothing "just works".
It means "mostly works most of the time, and occasionally goes horribly wrong"



For comparison, here is what (as I understand it) each PEP requires for a class 
to be used in a pattern such as `C(x, y, z)`:

```py
class C:
     """A PEP 634 matchable class."""
     __match_args__ = ...

class C:
     """A PEP 653 matchable class."""
     __match_kind__ = MATCH_CLASS
     __attributes__ = ...
     def __deconstruct__(self):
         ...
```

True, but most pure-Python classes don't have a natural order of attributes, so the pattern is likely to be `C(x=x,y=y,z=z)`.

In which case no extra work is required for either PEP to pattern match.


For a pattern with no positional subpatterns, such as like `C()` or `C(x=x, 
y=y, z=z)`:

```py
class C:
     """A PEP 634 matchable class (all classes work)."""

class C:
     """A PEP 653 matchable class."""
     __match_kind__ = MATCH_CLASS
     __attributes__ = ...
     def __deconstruct__(self):
         ...
```

This is not true. MATCH_DEFAULT does what you want.


It also appears that we lose a lot of expressive "idioms" by requiring 
`__attributes__` to be complete. For example, the following attribute extractions work 
with PEP 634, but not PEP 653.

```py
match subject:
     case object(a=_):
         # Match any object with an "a" attribute.
         ...
     case object(x=x, y=y, z=z):
         # Match any object with "x", "y", and "z" attributes, extracting them.
         ...
```

You need to define what you mean by "work" :)

This behaves the same for both PEPs:

class C:
    __init__(self, x, y, z):
        ...

match C(1,2,3):
    case object(x=x, y=y, z=z):
        ...

This also works the same for both PEPs, albeit with different mechanisms:

match namedtuple("n", "x, y, z")(1,2,3):
    case object(x=x, y=y, z=z):
        ...


But, with PEP 634:

>>> match(sympy.cos(x)):
>>>    case object(n):
>>>        ...
>>> n
<bound method EvalfMixin.evalf of cos(x)>

Which seem like "not working" to me.


This also means that matching classes like `types.SimpleNamespace` are much less powerful 
under PEP 653, since the class must know which attributes are "allowed" to be 
looked up.

Why do you say this?

This should work the same for PEP 634 and 653:

>>> match types.SimpleNamespace(a = 1):
>>>     case object(a):
>>>        print(a)
1


Further, this whole specification shifts most of the matching logic from the 
class in the pattern to the subject itself:

```py
match ChildA(), ChildB():
     case Parent(a, b), Parent(x, y):
         ...
```

The above pattern could have completely different rules for extracting `a` and 
`b` vs `x` and `y` if either child overrides `__deconstruct__`.  I think that 
is a mistake.

Inheritance allows sub-classes to change the behavior of their super-classes. Yes, it can produce weird effects if they ignore Liskov's substitution principle. That why the modern advice is "Favour composition over inheritance".

You can't prevent people writing bad code.


PEP 634 also privileges some builtin classes with a special form of matching, the 
"self" match. For example the pattern `list(x)` matches a list and assigns the 
list to `x`. By allowing classes to choose which kinds of pattern they match, other 
classes can use this form as well.

This is already fairly trivial to implement:

```py
class C:
     __match_args__ = ("_self",)
     _self = property(lambda s: s)
```

That's pretty obscure, IMO. You can't claim that it is trivial compared to:

class C:
    __match_kind__ = MATCH_SELF



You can even avoid adding a `_self` attribute if you do some magic with 
descriptors... ;). We could consider provide a decorator for this somewhere in 
the stdlib if there's demonstrated need (but I'm not sure there will be).

All classes should ensure that the the value of `__match_kind__` follows the 
specification. Therefore, implementations can assume, without checking, that 
all the following are false:
`(__match_kind__ & (MATCH_SEQUENCE | MATCH_MAPPING)) == (MATCH_SEQUENCE | 
MATCH_MAPPING)`
`(__match_kind__ & (MATCH_SELF | MATCH_CLASS)) == (MATCH_SELF | MATCH_CLASS)`
`(__match_kind__ & (MATCH_SELF | MATCH_DEFAULT)) == (MATCH_SELF | 
MATCH_DEFAULT)`
`(__match_kind__ & (MATCH_DEFAULT | MATCH_CLASS)) == (MATCH_DEFAULT | 
MATCH_CLASS)`

Oof. I was scratching my head for way too long at this before I noticed the previous 
sentence said "false". Maybe reword this section to indicate the conditions 
that hold *true*? :)

Good point. I'll do that.


Security Implications
Preventing the possible registering or unregistering of classes as sequences or 
a mappings, that PEP 634 allows, should improve security. However, the 
advantage is slight and is not a motivation for this PEP.

I would say the advantage is nonexistent. How is this any different than tweaking 
the flags in a class's `__match_kind__`? >
Efficient Implementation / Implementation

I won't get too deep into this section, but on the surface most flavors of 
implementation/optimization present here are also possible with PEP 634. I understand 
that you feel there are benefits to having "rules" for what optimizations are 
legal, but we don't need to completely change the mechanics of the match statement in 
order to do so.

Then please publish a precise semantics for PEP 634 pattern matching.

You say that "most flavors of implementation/optimization present here are also possible with PEP 634", but without semantics you can't show that optimizations preserve the semantics.


Because the object model is a core part of Python, implementations already 
handle special attribute lookup efficiently. Looking up a special attribute is 
much faster than performing a subclass test on an abstract base class.

But calling a `__deconstruct__` method whenever positional arguments are 
present will slow down normal class matches, right? I see it as mostly a wash.

Why do you think it will slow it down?
`__deconstruct__` is called only once. PEP 634, as written, has to perform a sequence of attribute lookups for *each* class pattern.

If a class defines `__deconstruct__`, it is likely to be for a reason.
For example, namedtuple's `__deconstruct__` would be:

def __deconstruct__(self):
    return self

Which is both simple and efficient.


The changes to the semantics can be summarized as:
- Selecting the kind of pattern uses `cls.__match_kind__` instead of 
`issubclass(cls, collections.abc.Mapping)` and `issubclass(cls, 
collections.abc.Sequence)` and allows classes control over which kinds of 
pattern they match.
- Class matching is via the `__attributes__` attribute and `__deconstruct__` 
method, rather than the `__match_args__` method, and allows classes more 
control over how they are deconstructed.

In general, it seems to me that this proposal prioritizes:

- complicated library code
- ease of implementation in the interpreter

over:

- simple client code
- general usability

It what way does this PEP make client code more complex, or impair usability? Evidence please.

"Ease of implementation in the interpreter" improves reliability
and performance. Those are important things in a language runtime :)



I can't say I support that, generally. When working on PEP 634, we tossed lot of more 
complicated, powerful protocols (like `__match__` and several others before it... don't 
mention the phrase "match proxy" around any of the authors!) in favor of 
simplicity. This sort of feels like a rehash of that.

If you had published these "more complicated, powerful protocols", you might be able to claim that this is a "rehash".
But you didn't.

Being vague is not "simplicity", it is merely hiding the complexity.

Cheers,
Mark.
_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/E3HFHCFTKA7QDHJRP7JDGE5TQU3H3ITE/
Code of Conduct: http://python.org/psf/codeofconduct/

Reply via email to