Skip to content
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

Named tuple's _replace() method should raise TypeError for unexpected keyword arguments #110275

Closed
serhiy-storchaka opened this issue Oct 3, 2023 · 4 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Oct 3, 2023

Bug report

When you call a function with incorrect key arguments, you get a TypeError. But it is not always so with the _replace() method of a named tuple class created by collections.namedtuple().

>>> from collections import namedtuple
>>> P = namedtuple('P', 'x y')
>>> p = P(1, 2)
>>> p._replace(z=3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/collections/__init__.py", line 460, in _replace
    raise ValueError(f'Got unexpected field names: {list(kwds)!r}')
ValueError: Got unexpected field names: ['z']

It is not even consistent with constructor which raises TypeError:

>>> P(x=1, y=2, z=3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: P.__new__() got an unexpected keyword argument 'z'

I think that _replace() also should raise TypeError for unexpected keyword arguments.

cc @rhettinger

Linked PRs

@serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Oct 3, 2023
@rhettinger
Copy link
Contributor

Let's leave these APIs alone. They are old, stable, and have never caused an issue for real users.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Oct 3, 2023
@serhiy-storchaka
Copy link
Member Author

Well, then I'll only change __replace__().

@rhettinger
Copy link
Contributor

Well, then I'll only change replace().

I wish you would just make the new code consistent with the existing replace implementations. Aim for the least invasive and least disruptive way to add your new function.

But if you're going to persist forcing through a TypeError, then go ahead and change _replace() as well. Better to have the two be harmonious and not introduce a permanent wart.

@serhiy-storchaka
Copy link
Member Author

What do you think about other changes that I took the opportunity to do in __replace__?

  1. Rename **kwds to **changes.
  2. Only include the first unexpected keyword argument name in the error message instead of the list. "Got unexpected field name: 'z'" instead of "Got unexpected field names: ['z']".

They are less important, so I do not persist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants