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.

classification
Title: tuple subclasses allow arbitrary kwargs
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: brandtbucher, jaraco, rhettinger, serhiy.storchaka
Priority: normal Keywords: 3.7regression, patch

Created on 2021-03-06 01:37 by jaraco, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 26456 merged serhiy.storchaka, 2021-05-30 16:02
PR 28403 merged serhiy.storchaka, 2021-09-17 06:03
Messages (13)
msg388183 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-03-06 01:37
While troubleshooting a strange problem (https://github.com/jaraco/keyring/issues/498) where a program worked on Python 3.7+ but failed on Python 3.6, I discovered a somewhat unintuitive behavior. On Python 3.7+, keyword arguments to tuple subclasses are allowed and ignored:

>>> class Foo(tuple): pass
>>> tuple(name='xyz')
TypeError: tuple() takes no keyword arguments
>>> Foo(name='xyz')
()

But on Python 3.6, the keyword parameter causes an error:

$ python3.6 -c "type('Foo', (tuple,), {})(name='xyz')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: 'name' is an invalid keyword argument for this function

I checked out the What's new in Python 3.7 and I see this notice:

Functions bool(), float(), list() and tuple() no longer take keyword arguments. The first argument of int() can now be passed only as positional argument.

Hmm, but in my experience, tuple on Python 3.6 doesn't take keyword arguments either:

importlib_metadata main $ python3.6 -c "tuple(name='xyz')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: 'name' is an invalid keyword argument for this function


So that change may be related, but I'm not sure where or how.

The main place my expectation is violated is in the subclass. Why should a subclass of tuple allow keyword arguments when the parent class does not? I'd expect that the subclass should reject keyword arguments as well.

Less importantly, the What's New doc implies that keyword arguments were accepted in Python 3.6; why aren't they?
msg388184 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-03-06 01:38
To be abundantly clear, the downstream issue was a coding error, but the coding error was masked on Python 3.7+ when the subclass didn't reject the invalid usage.
msg388186 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-03-06 01:56
I see that changelog entry traces back to bpo-29695, but I don't believe it's relevant to this issue.
msg388189 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-03-06 02:11
I suspect bpo-20186 is implicated.
msg388191 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-03-06 02:17
In particular, [this commit](https://github.com/python/cpython/commit/0b5615926a573c19c887a701a2f7047f4fd06de6).
msg388207 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-03-06 15:24
> Hmm, but in my experience, tuple on Python 3.6 doesn't take keyword arguments either:

They do.

>>> tuple(sequence='abc')
('a', 'b', 'c')

>>> list(sequence='abc')
['a', 'b', 'c']
>>> int(x='123')
123
>>> bool(x='123')
True
>>> float(x='123')
123.0

It was changed in bpo-29695.

But accepting arbitrary keyword arguments is another issue.

Object creation in Python can be customized be implementing special methods __new__ and __init__. They both are called sequentially with arguments passed to the constructor. If object is mutable, it is enough to implement __init__. If the object contains immutable data which should be initialized at creation time, it needs __new__, and __init__ is not necessary. So the list class has __init__, but the tuple and int classes have __new__. Usually class implements only one of __new__ or __init__, and inherits the other one from parent classes.

Since positional and keyword arguments are passed to both __new__ and __init__, they should accept same arguments. If __new__ and __init__ are inherited from parent class, they cannot know about arguments supported in the other method. Therefore object's __new__ and __init__ accept and ignore all arguments (if the other method is overridden).

Implementations of __new__ for most builtin classes which accept only positional arguments accept and ignore also arbitrary keyword arguments in subclasses. It makes easier to implement a subclass with non-trivial __init__. You just add additional keyword arguments which will be ignored in __new__. It is long tradition. Example:

    if (type == &cycle_type && !_PyArg_NoKeywords("cycle()", kwds))
        return NULL;

bpo-20186 just used this idiom for tuple and several other classes. It is a feature which makes subclassing these classes easier. And with converting more classes to Argument Clinic it is now used in more and more classes.

Now, perhaps it would be more correct to test `type->tp_init == cycle_type.tp_init` or `type->tp_init != PyBaseObject_Type.tp_init` instead of `type == &cycle_type`. It will ignore keyword arguments only if __init__ is overridden. If __init__ is overridden, it is now responsible for validating arguments.
msg394768 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-05-30 16:13
Surprisingly there were almost no tests for keyword arguments in subclasses.

PR 26456 makes checks for some classes (like tuple, list, frozenset) more strict: if subclass does not define __init__ or __new__, it will reject arbitrary keyword arguments.

It also makes the check in set.__init__() more lenient for uniformity with frozenset and list. Subclass of set can now define a __new__() method with additional keyword parameters without overriding also __init__().

Added tests for some of builtin classes.

Raymond, please take a look. It touches classes maintained tracked by you: set/frozenset, itertools, random.
msg401660 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-09-12 10:28
New changeset 92bf8691fb78f3484bf2daba836c416efedb1d8d by Serhiy Storchaka in branch 'main':
bpo-43413: Fix handling keyword arguments in subclasses of some buitin classes (GH-26456)
https://github.com/python/cpython/commit/92bf8691fb78f3484bf2daba836c416efedb1d8d
msg402000 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-09-17 03:58
> Subclass of set can now define a __new__() method with 
> additional keyword parameters without overriding also __init__().

Is there any use case for this?   Offhand, I can't think of any reason.

The new code in set.__init__ is somewhat opaque and is likely slower, so I prefer the previous code unless there is a legitimate use case being served.
msg402002 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-09-17 06:05
I do not have any particular use case. It was a side effect of unification code that uses _PyArg_NoKeywords().
msg402055 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-09-17 13:53
>> Subclass of set can now define
> Is there any use case for this?

Is your concern about a use-case for the general concept or for set specifically?

I appreciate that Serhiy has taken the time to evaluate the specific concern I raised and extrapolate it to the implications for most/all built-in types. It seems worthwhile to me that built-in types have consistent behaviors. Moreover, it seems preferable to provide the intuitive behavior (allowing simply overriding `__new__`) without the baggage of needing to define `__init__`. I'd not be surprised if there was a real-world use-case in which `set.__new__` was overridden in a subclass and the user was forced to add an `__init__` just to bypass "set() takes no keyword arguments". Interestingly, searching the web for that exact error doesn't emit any results, so if someone has encountered it, it wasn't posted with the error message.

After exploring this some, I'm convinced there may not be a strong case for this behavior.

Raymond, if this new behavior was removed, how would you propose to rewrite the test (specifically https://github.com/python/cpython/blob/778b07565e38cc94aa90318eb47b9cd09716756a/Lib/test/test_set.py#L665-L673)?
msg402056 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-09-17 13:55
Oh, and I see now Serhiy has proposed a change that looks reasonable.
msg409192 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-12-26 11:27
New changeset ad4857884b4821fc2c9bd23b63d03f9570eb03d1 by Serhiy Storchaka in branch 'main':
bpo-43413: Revert changes in set.__init__ (GH-28403)
https://github.com/python/cpython/commit/ad4857884b4821fc2c9bd23b63d03f9570eb03d1
History
Date User Action Args
2022-04-11 14:59:42adminsetgithub: 87579
2021-12-26 11:27:06serhiy.storchakasetmessages: + msg409192
2021-09-17 13:55:50jaracosetmessages: + msg402056
2021-09-17 13:53:47jaracosetmessages: + msg402055
2021-09-17 06:05:32serhiy.storchakasetmessages: + msg402002
2021-09-17 06:03:29serhiy.storchakasetpull_requests: + pull_request26815
2021-09-17 03:58:39rhettingersetmessages: + msg402000
2021-09-12 10:29:49serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-09-12 10:28:13serhiy.storchakasetmessages: + msg401660
2021-05-30 16:55:26serhiy.storchakasetnosy: + rhettinger
2021-05-30 16:13:51serhiy.storchakasetmessages: + msg394768
2021-05-30 16:02:45serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request25052
2021-05-28 08:34:58serhiy.storchakasetassignee: serhiy.storchaka
versions: + Python 3.11, - Python 3.7, Python 3.8, Python 3.9, Python 3.10
2021-05-28 01:16:04brandtbuchersetnosy: + brandtbucher
2021-05-27 01:21:42jaracosettitle: tuple subclasses allow kwargs -> tuple subclasses allow arbitrary kwargs
2021-03-06 15:24:04serhiy.storchakasetmessages: + msg388207
2021-03-06 02:46:24rhettingersetnosy: + serhiy.storchaka
2021-03-06 02:17:00jaracosetmessages: + msg388191
2021-03-06 02:11:42jaracosetmessages: + msg388189
2021-03-06 01:56:53jaracosetmessages: + msg388186
2021-03-06 01:38:40jaracosetmessages: + msg388184
2021-03-06 01:37:11jaracocreate