dataclasses should use NFKC to find duplicate members

[ericvsmith]

BPO	33881
Nosy	@rhettinger, @ericvsmith, @zooba, @srinivasreddy, @csabella, @ValeriyaSinevich
PRs	bpo-33881: Use NFKC to find duplicate members in make_dataclass #7916 bpo-33881: Use NFKC to find duplicate members in make_dataclass #20837

^{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:

assignee = 'https://github.com/ericvsmith'
closed_at = <Date 2020-06-13.16:42:00.859>
created_at = <Date 2018-06-16.18:02:08.301>
labels = ['3.8', 'library', '3.9', '3.10']
title = 'dataclasses should use NFKC to find duplicate members'
updated_at = <Date 2020-06-13.16:42:00.859>
user = 'https://github.com/ericvsmith'

bugs.python.org fields:

activity = <Date 2020-06-13.16:42:00.859>
actor = 'eric.smith'
assignee = 'eric.smith'
closed = True
closed_date = <Date 2020-06-13.16:42:00.859>
closer = 'eric.smith'
components = ['Library (Lib)']
creation = <Date 2018-06-16.18:02:08.301>
creator = 'eric.smith'
dependencies = []
files = []
hgrepos = []
issue_num = 33881
keywords = ['patch']
message_count = 8.0
messages = ['319766', '320446', '320454', '320455', '371356', '371424', '371435', '371463']
nosy_count = 6.0
nosy_names = ['rhettinger', 'eric.smith', 'steve.dower', 'thatiparthy', 'cheryl.sabella', 'valer']
pr_nums = ['7916', '20837']
priority = 'low'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue33881'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[ericvsmith]

See bpo-33880 for the same issue with namedtuple.

This shows up on dataclasses only through make_dataclass. This is an expected ValueError:

>>> make_dataclass('a', ['a', 'b', 'c', 'a'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/cygdrive/c/home/eric/.local/lib/python3.6/site-packages/dataclasses.py", line 1123, in make_dataclass
    raise TypeError(f'Field name duplicated: {name!r}')
TypeError: Field name duplicated: 'a'

But this is a SyntaxError:

>>> make_dataclass('a', ['\u00b5', '\u03bc'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/cygdrive/c/home/eric/.local/lib/python3.6/site-packages/dataclasses.py", line 1133, in make_dataclass
    unsafe_hash=unsafe_hash, frozen=frozen)
  File "/cygdrive/c/home/eric/.local/lib/python3.6/site-packages/dataclasses.py", line 958, in dataclass
    return wrap(_cls)
  File "/cygdrive/c/home/eric/.local/lib/python3.6/site-packages/dataclasses.py", line 950, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash, frozen)
  File "/cygdrive/c/home/eric/.local/lib/python3.6/site-packages/dataclasses.py", line 871, in _process_class
    else 'self',
  File "/cygdrive/c/home/eric/.local/lib/python3.6/site-packages/dataclasses.py", line 490, in _init_fn
    return_type=None)
  File "/cygdrive/c/home/eric/.local/lib/python3.6/site-packages/dataclasses.py", line 356, in _create_fn
    exec(txt, globals, locals)
  File "<string>", line 1
SyntaxError: duplicate argument 'μ' in function definition

[ValeriyaSinevich]

I sent a PR and measured how it affected the performance by creating a dataclass with 10000 members.
python.bat -m timeit -s "from dataclasses import make_dataclass" -s "arg_list = [chr(k) * i for k in range(97, 123) for i in range(1, 500)]" "make_dataclass('a', arg_list)"
The performance didn't change, both before and after the changes it takes around 14.5s.

[zooba]

The benchmark may not be triggering that much work. NFKC normalization only applies for characters outside of the basic Latin characters (0-255).

I ran the below benchmarks and saw a huge difference. Granted, it's a very degenerate case with collections this big, but it appears to be linear with len(NAMES), suggesting that the normalization is the expensive part.

>>> CHRS=[c for c in (chr(i) for i in range(65535)) if c.isidentifier()]
>>> def makename():
...  return ''.join(random.choice(CHRS) for _ in range(10))
...
>>> NAMES = [makename() for _ in range(10000)]
>>> timeit.timeit('len(set(NAMES))', globals=globals(), number=100000)
38.04007526000004
>>> timeit.timeit('len(set(unicodedata.normalize("NFKC", n) for n in NAMES))', globals=globals(), number=100000)
820.2586788580002

I wonder if it's better to catch the SyntaxError and do the check there? That way we don't really have a performance impact, since it's only going to show up in exceptional cases anyway.

[zooba]

outside of the basic Latin characters (0-255)

This should be (0-127). The Latin characters in 128-255 are considered extended ones, and may be decomposed by normalization.

[csabella]

The pull request that was opened for this has been closed due to inactivity, so this issue is available for anyone to work on.

[rhettinger]

I recommend leaving the code as-is. AFAICT, the situation almost never arises in practice, and if it did, the existing SyntaxError is clear.

Given that the rest of that language uses a SyntaxError, there isn't a net benefit for switching to TypeError:

>>> def f(µ=1, μ=2):
    	    pass
    	
SyntaxError: duplicate argument 'μ' in function definition
>>> def f(**kwds):
	return kwds

>>> f(µ=1, μ=2)
SyntaxError: keyword argument repeated

[srinivasreddy]

I am convinced by Raymond’s argument. Hence closing the PR.