Issue43413
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
Date: 2021-03-06 02:11 | |
I suspect bpo-20186 is implicated. |
|||
| msg388191 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2021-03-06 02:17 | |
In particular, [this commit](https://github.com/python/cpython/commit/0b5615926a573c19c887a701a2f7047f4fd06de6). |
|||
| msg388207 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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:42 | admin | set | github: 87579 |
| 2021-12-26 11:27:06 | serhiy.storchaka | set | messages: + msg409192 |
| 2021-09-17 13:55:50 | jaraco | set | messages: + msg402056 |
| 2021-09-17 13:53:47 | jaraco | set | messages: + msg402055 |
| 2021-09-17 06:05:32 | serhiy.storchaka | set | messages: + msg402002 |
| 2021-09-17 06:03:29 | serhiy.storchaka | set | pull_requests: + pull_request26815 |
| 2021-09-17 03:58:39 | rhettinger | set | messages: + msg402000 |
| 2021-09-12 10:29:49 | serhiy.storchaka | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2021-09-12 10:28:13 | serhiy.storchaka | set | messages: + msg401660 |
| 2021-05-30 16:55:26 | serhiy.storchaka | set | nosy:
+ rhettinger |
| 2021-05-30 16:13:51 | serhiy.storchaka | set | messages: + msg394768 |
| 2021-05-30 16:02:45 | serhiy.storchaka | set | keywords:
+ patch stage: patch review pull_requests: + pull_request25052 |
| 2021-05-28 08:34:58 | serhiy.storchaka | set | assignee: serhiy.storchaka versions: + Python 3.11, - Python 3.7, Python 3.8, Python 3.9, Python 3.10 |
| 2021-05-28 01:16:04 | brandtbucher | set | nosy:
+ brandtbucher |
| 2021-05-27 01:21:42 | jaraco | set | title: tuple subclasses allow kwargs -> tuple subclasses allow arbitrary kwargs |
| 2021-03-06 15:24:04 | serhiy.storchaka | set | messages: + msg388207 |
| 2021-03-06 02:46:24 | rhettinger | set | nosy:
+ serhiy.storchaka |
| 2021-03-06 02:17:00 | jaraco | set | messages: + msg388191 |
| 2021-03-06 02:11:42 | jaraco | set | messages: + msg388189 |
| 2021-03-06 01:56:53 | jaraco | set | messages: + msg388186 |
| 2021-03-06 01:38:40 | jaraco | set | messages: + msg388184 |
| 2021-03-06 01:37:11 | jaraco | create | |
