tuple subclasses allow arbitrary kwargs

[jaraco]

BPO	43413
Nosy	@rhettinger, @jaraco, @serhiy-storchaka, @brandtbucher
PRs	bpo-43413: Fix handling keyword arguments in subclasses of some buitin classes #26456 bpo-43413: Revert changes in set.__init__ #28403

^{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/serhiy-storchaka'
closed_at = <Date 2021-09-12.10:29:49.494>
created_at = <Date 2021-03-06.01:37:11.195>
labels = ['interpreter-core', '3.11']
title = 'tuple subclasses allow arbitrary kwargs'
updated_at = <Date 2021-12-26.11:27:06.889>
user = 'https://github.com/jaraco'

bugs.python.org fields:

activity = <Date 2021-12-26.11:27:06.889>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2021-09-12.10:29:49.494>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2021-03-06.01:37:11.195>
creator = 'jaraco'
dependencies = []
files = []
hgrepos = []
issue_num = 43413
keywords = ['patch', '3.7regression']
message_count = 13.0
messages = ['388183', '388184', '388186', '388189', '388191', '388207', '394768', '401660', '402000', '402002', '402055', '402056', '409192']
nosy_count = 4.0
nosy_names = ['rhettinger', 'jaraco', 'serhiy.storchaka', 'brandtbucher']
pr_nums = ['26456', '28403']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue43413'
versions = ['Python 3.11']

[jaraco]

While troubleshooting a strange problem (jaraco/keyring#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?

[jaraco]

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.

[jaraco]

I see that changelog entry traces back to bpo-29695, but I don't believe it's relevant to this issue.

[jaraco]

I suspect bpo-20186 is implicated.

[jaraco]

In particular, this commit.

[serhiy-storchaka]

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.

[serhiy-storchaka]

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.

[serhiy-storchaka]

New changeset 92bf869 by Serhiy Storchaka in branch 'main':
bpo-43413: Fix handling keyword arguments in subclasses of some buitin classes (GH-26456)
92bf869

[rhettinger]

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.

[serhiy-storchaka]

I do not have any particular use case. It was a side effect of unification code that uses _PyArg_NoKeywords().

[jaraco]

> 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

cpython/Lib/test/test_set.py

Lines 665 to 673 in 778b075

	class subclass_with_new(set):
	def __new__(cls, arg, newarg=None):
	self = super().__new__(cls, arg)
	self.newarg = newarg
	return self
	u = subclass_with_new([1, 2], newarg=3)
	self.assertIs(type(u), subclass_with_new)
	self.assertEqual(set(u), {1, 2})
	self.assertEqual(u.newarg, 3)

)?

[jaraco]

Oh, and I see now Serhiy has proposed a change that looks reasonable.

[serhiy-storchaka]

New changeset ad48578 by Serhiy Storchaka in branch 'main':
bpo-43413: Revert changes in set.__init__ (GH-28403)
ad48578