On Jun 23, 2020, at 09:01, Guido van Rossum email@example.com wrote:
I'm happy to present a new PEP for the python-dev community to review. This is joint work with Brandt Bucher, Tobias Kohn, Ivan Levkivskyi and Talin.
Congratulations to the authors. This is a well written, complex PEP for a powerful feature. Here are some of my initial comments/questions.
Couldn’t you adopt a flat indentation scheme with the minor change of moving the expression into a `match:` clause? E.g.
match: expression case a: foo() case b: bar() else: baz()
I didn’t see that in the rejected alternatives.
I’m with others who think that `else:` would be a better choice than `case _:`. Given my background in i18n (see the flufl.i18n library, etc.), my red flags go up when I see bare underscores being given syntactic meaning.
For example, the restriction against `case _.a:` *could* interact badly with my library. There, _ is the most common name binding for an object that implements the translation semantics. Therefore, it has attributes. I can’t think of a concrete example right now, but e.g. what if I wanted to match against `_.code`? That wouldn’t be legal if I’m understanding correctly (`code` being an attribute of the object typically bound to _).
I’m also concerned about the .BLACK vs BLACK example. I get why the distinction is there, and I get that the PEP proposes that static analyzers help the ambiguity, but it strikes me as a potential gotcha and a source of mysterious errors.
Why not just bare `*` instead of `*_` in patterns?
The PEP is unclear about what kind of method __match__() is. As I was reading along, I suspected it must be a static or class method, explicitly cannot be an instance method because the class in the case statement is never instantiated, but it’s not until I got to the object.__match__() discussion that this distinction was made clear. I think the PEP should just be explicit upfront about that.
As a side note, I’m also concerned that `case Point(x, y)` *looks* like it instantiates `Point` and that it’s jarring to Python developers that they have to mentally switch models when reading that code.
I was also unclear about that __match__() had to return an object until much later in the PEP. My running notes asked:
# Is returning None better than raising an exception? This won’t work: class C: def __init__(self, x=None): self.x = x @staticmethod def __match__(obj): # Should just return obj? return obj.x
match C(): case x: print(x)
But once I read on, I realized that __match__() should return `obj` in this case. Still, the fact that returning None to signal a case arm not matching feels like there’s a gotcha lurking in there somewhere.
Should @sealed be in a separate PEP? It’s relevant to the discussion in 622, but seems like it would have use outside of the match feature so should possibly be proposed separately.
The PEP is unclear whether `case` is also a soft keyword. I’m guessing it must be.
Those are my current thoughts on first read through.