classification
Title: Constructors of some mapping classes don't accept `self` keyword argument
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: abacabadabacaba, berker.peksag, larry, mark.dickinson, martin.panter, python-dev, rhettinger, serhiy.storchaka, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2014-10-11 12:04 by abacabadabacaba, last changed 2015-09-29 20:59 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
collections_pos_only_params.patch serhiy.storchaka, 2014-10-11 14:12 review
collections_pos_only_params2.patch serhiy.storchaka, 2014-10-12 06:57 review
UserDict_self_and_dict_keywords.patch serhiy.storchaka, 2015-03-25 18:46 review
UserDict_self_and_dict_keywords_2.patch serhiy.storchaka, 2015-03-31 05:28 review
UserDict_self_and_dict_keywords_3.patch serhiy.storchaka, 2015-06-26 17:59 review
Messages (34)
msg229076 - (view) Author: Evgeny Kapun (abacabadabacaba) Date: 2014-10-11 12:04
>>> import collections
>>> collections.Counter(self=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init__() got multiple values for argument 'self'
>>> collections.OrderedDict(self="test")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init__() got multiple values for argument 'self'
>>> collections.UserDict(self="test")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init__() got multiple values for argument 'self'

Python surely should have positional-only parameters.
msg229089 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-10-11 14:12
Here is a patch.
msg229098 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-10-11 17:10
I'll spend some time taking this one under consideration.

Keyword arguments for dicts and dict-like classes are already somewhat limited (only non-keyword identifiers) that why Guido resisted adding them in the first place.   And, in the context of OrderedDicts, they aren't really useful at all (because the order gets scrambled).

I'm reluctant to make the number of changes required in order to support this corner case.  The ship for Python 2.7 sailed a long time ago and it is risky to make changes like this.  After the patch, the code is less readable, harder to get right, harder to maintain, and not as performant.

On the other hand, adding "self" would be a nice-to-have, so it is worth thinking about.
msg229118 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-10-12 06:15
After some thought, I think we have to fix this.

I'll go through the patch in careful detail this weekend.

Ethan, if you would like to help, it would be great to have a third pair of eyes looking at the patch to make sure it correct it every detail.
msg229121 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-10-12 06:57
I thought that more verbose but straightforward code would be more acceptable. 
Well, here is smaller and clever patch. Tests are the same.
msg229189 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-10-12 18:43
I will take a look.
msg229248 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-10-13 15:27
Code looks good.

Only downside is the change in help and inspect.signature output, but that is minor:

Help on dict object:
class dict(object)
 [...]
|  __init__(self, /, *args, **kwargs)

  vs.

Help on class Counter in module collections:
class Counter(builtins.dict)
 [...]
|  __init__(*args, **kwds)

+1 to accept.
msg229249 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-10-13 16:32
FWIW, I agree that it should be fixed:

>>> dict(self=1)
{'self': 1}
msg231753 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-11-27 06:47
This looks good.  Go ahead and apply the first version of the patch.
msg231763 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-11-27 14:54
New changeset 816c15fe5812 by Serhiy Storchaka in branch '3.4':
Issue #22609: Constructors and update methods of mapping classes in the
https://hg.python.org/cpython/rev/816c15fe5812

New changeset 88ab046fdd8a by Serhiy Storchaka in branch 'default':
Issue #22609: Constructors and update methods of mapping classes in the
https://hg.python.org/cpython/rev/88ab046fdd8a
msg231765 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-11-27 15:51
New changeset cd1ead4feddf by Serhiy Storchaka in branch '3.4':
Issue #22609: Revert changes in UserDict. They conflicted with existing tests.
https://hg.python.org/cpython/rev/cd1ead4feddf

New changeset 167d51a54de2 by Serhiy Storchaka in branch 'default':
Issue #22609: Revert changes in UserDict. They conflicted with existing tests.
https://hg.python.org/cpython/rev/167d51a54de2
msg231766 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-11-27 16:00
Unfortunately there is existing test for current behavior of UserDict:

    self.assertEqual(collections.UserDict(dict=[('one',1), ('two',2)]), d2)

This looks wrong to me and I think we should break compatibility here.
msg231769 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-11-27 17:05
New changeset 3dfe4f0c626b by Serhiy Storchaka in branch '2.7':
Issue #22609: Constructors and update methods of mapping classes in the
https://hg.python.org/cpython/rev/3dfe4f0c626b
msg231915 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-12-01 08:58
So what to do wish UserDict? Should we break backward compatibility and add support for "self" and "dict" keywords as in other dict-like types?
msg231936 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-12-01 12:42
Fix `self` now, add a warning and single minor cycle deprecation period for 'dict'.
msg232353 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-12-09 08:38
[Serhiy]
> So what to do wish UserDict?

I'm leaning in favor of leaving UserDict as-is.   AFAICT, in the very long history of UserDict, this has never been a problem.  So, I don't think there is an issue worth breaking the published API and possibly breaking code that currently works.  (This is a case of practicality beating purity).
msg234875 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-28 09:02
So may be close this issue?

See also issue22958.
msg239278 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-25 18:46
Here is a patch for UserDict, that keep current behavior, but allows to pass keys "self" and "dict" if positional parameter dict is specified.

>>> UserDict(self=42)
{'self': 42}
>>> UserDict({}, dict=42)
{'dict': 42}
>>> UserDict(dict={'a': 42})
{'a': 42}
msg239655 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-03-31 02:19
UserDict_self_and_dict_keywords.patch looks good to me. Is there a reason not to use assertWarnsRegex?

Also, there are already collections.UserDict() usages in the test file, so I'd remove the "from collections import UserDict" import.
msg239667 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-31 05:28
Thank you for your review Berker.

> Is there a reason not to use assertWarnsRegex?

Initially the patch was written for 2.7. Fixing WeakValueDictionary in 2.7 
needs first fix UserDict (issue22958). That is why I returned to this issue.

> Also, there are already collections.UserDict() usages in the test file, so
> I'd remove the "from collections import UserDict" import.

These tests originally was written for test_collections.

Updated patch addresses Berker's comment.
msg243329 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-05-16 16:11
Could you please look at the new patch Raymond? This is the dependency for issue22958.
msg244470 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-05-30 16:17
Ping.
msg244503 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-05-31 00:20
Left a couple pedantic comments.
msg245859 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-06-26 17:59
Updated patch addresses Martin's comments.
msg245877 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-06-27 02:49
Left some feedback in the code review.
msg245944 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-06-29 15:35
Patch looks fine to me. I understand Yury withdrew his comment.
msg246936 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-07-19 10:16
Ping again.
msg246954 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-07-20 04:20
Who are you pinging? I did just notice a minor English grammar problem (“one arguments”). But as far as I am concered you could have already committed the patch.
msg246956 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-07-20 04:46
I will look at this more when I get a chance (likely this week).
msg246957 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-07-20 05:11
I was pinging Raymond. He is maintainer of the collections module, this issue is assigned to his, and he had valid objections to previous version of the patch. Even one of this reason is enough to wait his review before committing.

Thank you Raymond.
msg251778 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-28 17:12
Ping.
msg251831 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-29 09:41
UserDict_self_and_dict_keywords_3.patch looks good to me.
msg251886 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-09-29 20:54
New changeset 4c5407e1b0ec by Serhiy Storchaka in branch '2.7':
Issue #22609: Constructor and the update method of collections.UserDict now
https://hg.python.org/cpython/rev/4c5407e1b0ec

New changeset 1869f5625392 by Serhiy Storchaka in branch '3.4':
Issue #22609: Constructor of collections.UserDict now accepts the self keyword
https://hg.python.org/cpython/rev/1869f5625392

New changeset ab7e3f1f9f88 by Serhiy Storchaka in branch '3.5':
Issue #22609: Constructor of collections.UserDict now accepts the self keyword
https://hg.python.org/cpython/rev/ab7e3f1f9f88

New changeset 901964295066 by Serhiy Storchaka in branch 'default':
Issue #22609: Constructor of collections.UserDict now accepts the self keyword
https://hg.python.org/cpython/rev/901964295066
msg251888 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-29 20:59
Thank you all for your reviews.
History
Date User Action Args
2015-09-29 20:59:53serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg251888

stage: commit review -> resolved
2015-09-29 20:54:28python-devsetmessages: + msg251886
2015-09-29 09:41:01vstinnersetnosy: + vstinner
messages: + msg251831
2015-09-28 17:12:54serhiy.storchakasetmessages: + msg251778
2015-07-21 07:02:52ethan.furmansetnosy: - ethan.furman
2015-07-20 05:11:18serhiy.storchakasetmessages: + msg246957
2015-07-20 04:46:33rhettingersetmessages: + msg246956
2015-07-20 04:20:01martin.pantersetmessages: + msg246954
2015-07-19 10:16:54serhiy.storchakasetmessages: + msg246936
2015-06-29 15:35:13martin.pantersetmessages: + msg245944
stage: patch review -> commit review
2015-06-27 02:49:34yselivanovsetnosy: + yselivanov
messages: + msg245877
2015-06-26 17:59:00serhiy.storchakasetfiles: + UserDict_self_and_dict_keywords_3.patch

messages: + msg245859
versions: + Python 3.6
2015-05-31 00:20:34martin.pantersetnosy: + martin.panter
messages: + msg244503
2015-05-30 16:17:19serhiy.storchakasetmessages: + msg244470
2015-05-16 16:11:32serhiy.storchakasetmessages: + msg243329
2015-05-16 16:11:09serhiy.storchakalinkissue22958 dependencies
2015-03-31 05:28:50serhiy.storchakasetfiles: + UserDict_self_and_dict_keywords_2.patch

messages: + msg239667
2015-03-31 02:19:03berker.peksagsetnosy: + berker.peksag
messages: + msg239655
2015-03-25 18:46:52serhiy.storchakasetfiles: + UserDict_self_and_dict_keywords.patch

messages: + msg239278
2015-01-28 09:02:09serhiy.storchakasetmessages: + msg234875
2014-12-09 08:38:52rhettingersetmessages: + msg232353
2014-12-01 17:08:35rhettingersetpriority: high -> normal
assignee: serhiy.storchaka -> rhettinger
2014-12-01 12:42:20ethan.furmansetmessages: + msg231936
2014-12-01 08:58:50serhiy.storchakasetmessages: + msg231915
2014-11-27 17:05:38python-devsetmessages: + msg231769
2014-11-27 16:00:51serhiy.storchakasetmessages: + msg231766
2014-11-27 15:51:03python-devsetmessages: + msg231765
2014-11-27 14:54:56python-devsetnosy: + python-dev
messages: + msg231763
2014-11-27 06:47:59rhettingersetassignee: rhettinger -> serhiy.storchaka
messages: + msg231753
2014-11-01 03:41:42rhettingersetpriority: normal -> high
2014-10-13 16:32:26larrysetnosy: + larry
messages: + msg229249
2014-10-13 15:27:46ethan.furmansetmessages: + msg229248
2014-10-12 18:43:49ethan.furmansetmessages: + msg229189
2014-10-12 06:57:29serhiy.storchakasetfiles: + collections_pos_only_params2.patch

messages: + msg229121
2014-10-12 06:15:07rhettingersetpriority: low -> normal

messages: + msg229118
2014-10-11 17:10:02rhettingersetpriority: normal -> low

messages: + msg229098
2014-10-11 16:56:07rhettingersetassignee: rhettinger
2014-10-11 14:12:17serhiy.storchakasetfiles: + collections_pos_only_params.patch

versions: + Python 2.7, Python 3.5
keywords: + patch
nosy: + rhettinger, serhiy.storchaka

messages: + msg229089
stage: patch review
2014-10-11 12:26:52ethan.furmansetnosy: + ethan.furman
2014-10-11 12:20:17mark.dickinsonsetnosy: + mark.dickinson
2014-10-11 12:04:09abacabadabacabacreate