This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Created on 2021-07-09 10:16 by quentel, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 27131 merged jack__d, 2021-07-14 03:35
PR 27157 merged miss-islington, 2021-07-15 00:38
Messages (12)
msg397195 - (view) Author: Pierre Quentel (quentel) * Date: 2021-07-09 10:16
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}:
msg397203 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2021-07-09 17:06
Nice find! In my opinion, we should do two things here: - Update PEP 638 to specify that a SyntaxError is raised "if *any duplicate* key patterns are literal patterns". This was absolutely our original intention... I think the nuances were just lost in the final phrasing. I can take care of that PR. - Update the compiler to raise SyntaxErrors for duplicate literal keys. It should be as simple as updating the first loop in compiler_pattern_mapping to build a set containing the values of any literal keys and raise on duplicates (and adding/updating tests, of course). We'll want to make sure we have test coverage of edge cases like {0: _, False: _}, {0: _, 0.0: _}, {0: _, -0: _}, {0: _, 0j: _}, etc. Since you found this, you get first dibs on a PR. Otherwise, I have a few first-time contributors who would probably be interested.
msg397204 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2021-07-09 17:07
Sorry, that should be PEP 634, not 638.
msg397215 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-07-09 20:10
Agreed with everything that Brandt said. These wording issues are subtle! Note in particular that the original wording implies that {‘x’: 1, ‘x’: 1, z: 1} would be a runtime error, but it clearly should be a compile time error.-- --Guido (mobile)
msg397240 - (view) Author: Pierre Quentel (quentel) * Date: 2021-07-10 05:41
Sorry, I don't know C so I can't write a PR for this change.
msg397460 - (view) Author: Jack DeVries (jack__d) * Date: 2021-07-14 04:02
@brandtbucher, I think that my PR implements the solution you've described here. What do you think?
msg397493 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2021-07-14 16:23
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. :)
msg397498 - (view) Author: Jack DeVries (jack__d) * Date: 2021-07-14 17:38
@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.
msg397519 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2021-07-15 00:38
 New changeset 2693132292b2acf381ac6fa729bf3acf41d9d72b by Jack DeVries in branch 'main': bpo-44589: raise a SyntaxError when mapping patterns have duplicate literal keys (GH-27131) https://github.com/python/cpython/commit/2693132292b2acf381ac6fa729bf3acf41d9d72b 
msg397522 - (view) Author: miss-islington (miss-islington) Date: 2021-07-15 01:00
 New changeset 016af14d93cfba43e7a95721a97fa954c534af8e by Miss Islington (bot) in branch '3.10': [3.10] bpo-44589: raise a SyntaxError when mapping patterns have duplicate literal keys (GH-27131) (GH-27157) https://github.com/python/cpython/commit/016af14d93cfba43e7a95721a97fa954c534af8e 
msg397687 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-07-17 03:38
Is something left here?
msg397697 - (view) Author: Jack DeVries (jack__d) * Date: 2021-07-17 04:16
The PR and backport to 3.10 have both been merged, so I think this issue can be closed.
History
Date User Action Args
2022-04-11 14:59:47adminsetgithub: 88755
2021-07-17 04:20:16pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-07-17 04:16:50jack__dsetmessages: + msg397697
2021-07-17 03:38:22pablogsalsetnosy: + pablogsal
messages: + msg397687
2021-07-15 01:00:54miss-islingtonsetmessages: + msg397522
2021-07-15 00:38:57brandtbuchersetmessages: + msg397519
2021-07-15 00:38:51miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request25694
2021-07-14 17:38:00jack__dsetmessages: + msg397498
2021-07-14 16:23:24brandtbuchersetmessages: + msg397493
2021-07-14 04:02:03jack__dsetmessages: + msg397460
2021-07-14 03:35:17jack__dsetkeywords: + patch
nosy: + jack__d

pull_requests: + pull_request25674
stage: patch review
2021-07-10 19:33:14brandtbuchersetassignee: brandtbucher
2021-07-10 05:41:41quentelsetmessages: + msg397240
2021-07-09 20:10:56gvanrossumsetmessages: + msg397215
2021-07-09 17:07:04brandtbuchersetmessages: + msg397204
2021-07-09 17:06:33brandtbuchersetversions: + Python 3.11, - Python 3.10
nosy: + gvanrossum

messages: + msg397203

type: behavior
2021-07-09 11:24:36xtreaksetnosy: + brandtbucher
2021-07-09 10:16:54quentelcreate