Hi Brandt,
On 02/04/2021 7:19 am, Brandt Bucher wrote:
> Guido van Rossum wrote:
>> Well, now I have egg on my face, because the current implementation does reject multiple occurrences of the same identifier in __match_args__. We generate an error like "TypeError: C() got multiple sub-patterns for attribute 'a'". However, I cannot find this uniqueness requirement in PEP 634, so I think it was a mistake to implement it.
>>
>> Researching this led me to find another issue where PEP 634 and the implementation differ, but this time it's the other way around: PEP 634 says about types which accept a single positional subpattern (int(x), str(x) etc.) "for these types no keyword patterns are accepted." Mark's example `case int(real=0, imag=0):` makes me think this requirement is wrong and I would like to amend PEP 634 to strike this requirement. Fortunately, this is not what is implemented. E.g. `case int(1, real=1):` is accepted and works, as does `case int(real=0):`.
>>
>> Calling out Brandt to get his opinion. And thanks to Mark for finding these!
>
> The current implementation will reject any attribute being looked up more than once, by position *or* keyword. It's actually a bit tricky to do, which is why the `MATCH_CLASS` op is such a beast... it needs to look up positional and keyword attributes all in one go, keeping track of everything it's seen and checking for duplicates.
>
> I believe this behavior is a holdover from PEP 622:
>
>> The interpreter will check that two match items are not targeting the same attribute, for example `Point2d(1, 2, y=3)` is an error.
>
> (https://www.python.org/dev/peps/pep-0622/#overlapping-sub-patterns)
>
> PEP 634 explicitly disallows duplicate keywords, but as far as I can tell it says nothing about duplicate `__match_args__` or keywords that also appear in `__match_args__`. It looks like an accidental omission during the 622 -> 634 rewrite.
>
> (I guess I figured that if somebody matches `Spam(foo, y=bar)`, where `Spam.__match_args__` is `("y",)`, that's probably a bug in the user's code. Ditto for `Spam(y=foo, y=bar)` and `Spam(foo, bar)` where `Spam.__match_args__` is `("y", "y")` But it's not a hill I'm willing to die on.)
Repeated keywords do seem likely to be a bug.
Agreed. But as I sketched in a previous email I think duplicates ought to be acceptable in __match_args__. At the very least we should align the PEP and the implementation here, by adjusting one or the other.
Most checks are cheap though.
Checking for duplicates in `__match_args__` can be done at class
creation time,
Hm, what about dynamic updates to __match_args__? I've done that in the REPL.
and checking for duplicates in the pattern can be done at
compile time.
I'd prefer not to do that check at all.
So how about explicitly disallowing those, but not checking that the
intersection of `__match_args__` and keywords is empty?
We would get most of the error checking without the performance impact.
+1 on the latter (not checking the intersection).
>
> I agree that self-matching classes should absolutely allow keyword matches. I had no idea the PEP forbade it.
PEP 634 allows it. PEP 653 currently forbids it, mainly for consistency
reasons.
The purpose of self-matching is to prevent deconstruction, so it seems
inconsistent to allow it for keyword arguments.
The purpose of self-matching is user convenience. It should be seen as a shorthand for the code fragment in PEP 634 showing how to do this for any class.
Are there are any use-cases?
The test-case `int(real=0+0j, imag=0-0j)` is contrived,
but I'm struggling to come up with less contrived examples for any of
float, list, dict, tuple, str.
There could be a subclass that adds an attribute. That's still contrived though. But if we start supporting this for *general* classes we should allow combining it with keywords/attributes.