New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pattern Matching - duplicate keys in mapping patterns #88755
Comments
PEP-634 specifies that "A mapping pattern may not contain duplicate key values. (If all key patterns are literal patterns this is considered a syntax error; otherwise this is a runtime error and will raise ValueError.)" but this is not what happens with the latest release: Python 3.10.0b3 (tags/v3.10.0b3:865714a, Jun 17 2021, 20:39:25) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> x = {'a': 1}
>>> match x:
... case {'a': 1, 'a': 2}: # (A)
... print('ok')
...
>>> x = {'a': 3}
>>> match x:
... case {'a': 1, 'a': 2}: # (B)
... print('ok')
...
>>> x = {'a': 1, 'b': 2}
>>> match x:
... case {'a': 1, 'a': 2}: # (C)
... print('ok')
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
ValueError: mapping pattern checks duplicate key ('a')
>>> If I understand the PEP correctly, all these examples should raise a SyntaxError for the line case {'a': 1, 'a': 2}: since all key patterns are literal patterns, and the key 'a' is duplicated. Cases (A) where the subject matches one of the key-value patterns, and (B) when it doesn't, fail without raising SyntaxError. Case (C) where one of the keys in the subject is not present in the mapping pattern raises a ValueError at runtime instead of SyntaxError. This behaviour is tested in test_patma.py: def test_patma_251(self):
x = {"a": 0, "b": 1}
w = y = z = None
with self.assertRaises(ValueError):
match x:
case {"a": y, "a": z}:
w = 0
self.assertIs(w, None)
self.assertIs(y, None)
self.assertIs(z, None) but this doesn't seem compliant with the specification. BTW, it's not clear to me why the SyntaxError should be limited to the case when all keys are literal patterns; it could be raised whenever a literal pattern is repeated, even when there are value patterns or a double-star pattern, like in case {'a': 1, 'a': 2, c.imag, **rest}: |
Nice find! In my opinion, we should do two things here:
Since you found this, you get first dibs on a PR. Otherwise, I have a few first-time contributors who would probably be interested. |
Sorry, that should be PEP-634, not 638. |
Agreed with everything that Brandt said. These wording issues are subtle! |
Sorry, I don't know C so I can't write a PR for this change. |
@brandtbucher, I think that my PR implements the solution you've described here. What do you think? |
Thanks for the patch @jack__d! I'll try to find time to review it today. I do wish you would have coordinated with me here before writing it, though. I'd already begun working on a patch with a few new contributors yesterday, as I mentioned in my comment. In the future, please check in with those involved in the PR discussion first (especially if the issue is already "assigned"). We're doing this for fun, too. :) |
@brandtbucher, I'm sorry for the miscommunication. I started working on this patch at the beginning of the week after message 397215... I'm a new contributor too, and I wasn't sure if I would be able to make something that worked, so I just kept my mouth shut. Then, all of a sudden, it came together and I had a (mostly) working patch. I'm going to incorporate your review now; thank you for the feedback, and I'm very sorry for any toes I may have stepped on. |
Is something left here? |
The PR and backport to 3.10 have both been merged, so I think this issue can be closed. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: