| msg183260 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2013-03-01 13:45 |
| The functionality of dict.setdefault() is not currently available through the C-API. Specfically, there is no way to test for a key and insert a fallback value for it without evaluating the hash function twice. The attached patch adds a new C-API function PyDict_GetItemSetDefault() that makes this feature available. Like all PyDict_Getitem*() functions, it returns a borrowed reference. I hope I got the update in refcounts.dat right. I have no idea how to express that a function *may* increase the refcounts of its arguments. |
|
|
| msg183638 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-03-07 08:10 |
| +1 for the idea. Please change "failobj" to "default" to keep the terminology consistent with the pure Python API. For reference, here's the code from collections.MutableMapping: def setdefault(self, key, default=None): try: return self[key] except KeyError: self[key] = default return default |
|
|
| msg183662 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-07 14:05 |
| The problem with 'default' is that it is a reserved word in C. I changed it to "defaultobj", except for the docs page, where "default" should work. I also removed the "register" declaration from the "mp" argument because it is most likely useless and just takes up screen real estate. |
|
|
| msg183695 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2013-03-07 19:01 |
| Please call it PyDict_SetDefault, though. |
|
|
| msg183696 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-07 19:06 |
| I had originally considered that name. However, what it really does is PyDict_GetItem(). In a specific special case, it sets a default value and *then* returns that. So it's still PyDict_GetItem(), just with a preceding modification. Also, the interface mimics PyDict_GetItem(). Its primary interface is a getter, not a setter. I really think that PyDict_SetDefault() is an inappropriate name. |
|
|
| msg183697 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2013-03-07 19:12 |
| PyDict_SetDefault mimicks the Python API, though. |
|
|
| msg183698 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-07 19:15 |
| Well, guess what, I kind-of figured that. So, what's wrong with PyDict_GetItemSetDefault()? That mimics the Python method name, too, while at the same time making it clear what actually happens and how the C-API function behaves. |
|
|
| msg183699 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-03-07 19:18 |
| I would simply call it PyDict_SetDefault, too. It's also shorter to type ;) |
|
|
| msg183700 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-07 19:21 |
| To me, PyDict_SetDefault() sounds like it's supposed to set a default value that PyDict_GetItem() would return instead of NULL on lookup failure. Basically a defaultdict-like extension to normal dicts. |
|
|
| msg183701 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2013-03-07 19:25 |
| I find the name PyDict_GetItemSetDefault very confusing. My brain isn't quite sure where to partition the four words of GetItemSetDefault. If you really wanted to be clear, you would have to call it PyDict_GetItemAndSetDefault. There's no reason not to pick the same name as the Python-level API, though. |
|
|
| msg183702 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-07 19:27 |
| I'm fine with PyDict_GetItemOrSetDefault() as well. |
|
|
| msg183710 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-03-07 23:10 |
| +1 for PyDict_SetDefault. Any other name is confusing. |
|
|
| msg183713 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-07 23:27 |
| Ok (shrug), since everyone seems to agree, PyDict_SetDefault() it is. I wouldn't be surprised if the same kind of discussion lead to the original naming of dict.setdefault()... |
|
|
| msg183723 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-03-08 03:17 |
| New changeset a0b750ea3397 by Benjamin Peterson in branch 'default': Add PyDict_SetDefault. (closes #17327) http://hg.python.org/cpython/rev/a0b750ea3397 |
|
|
| msg183728 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-08 07:00 |
| If you decide to refactor a well tested patch, you might want to give it another test run before committing it. Objects/dictobject.c: In function 'dict_setdefault': Objects/dictobject.c:2266:5: warning: passing argument 1 of 'PyDict_SetDefault' from incompatible pointer type [enabled by default] Objects/dictobject.c:2215:1: note: expected 'struct PyObject *' but argument is of type 'struct PyDictObject *' |
|
|
| msg183729 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-08 07:48 |
| I'm totally ok with your changes, though. The only real difference is an aditional type check in the slot function path, and that's not going to make any difference right after the costly operation of unpacking python function arguments. |
|
|
| msg183736 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-03-08 13:36 |
| New changeset ca9a85c36e09 by Benjamin Peterson in branch 'default': fix warning (closes #17327) http://hg.python.org/cpython/rev/ca9a85c36e09 |
|
|
| msg183952 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-11 11:41 |
| Benjamin, I hadn't noticed that you also changed the documentation in dict.rst from what I originall wrote. Ezio already fixed one of your typos, but there's another one in the description of the "defaultobj" behaviour: it says "inserted" twice but should say "returned" in the second case. Please give it another read to see if you introduced any further errors. |
|
|
| msg183971 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-03-11 16:31 |
| New changeset 89174df2ee45 by Benjamin Peterson in branch 'default': remove useless words (#17327) http://hg.python.org/cpython/rev/89174df2ee45 |
|
|
| msg183974 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-11 16:44 |
| Please use either of the following wordings in the documentation: """ If the key is not in the dict, it is inserted with value *defaultobj* and *defaultobj* is returned. """ or """ If the key is not in the dict, it is inserted with value *defaultobj*, which is then returned. """ Otherwise, the docs leave it open if anything at all is returned in the case of a lookup failure. (I initially wanted to make that clear with the PyDict_GetItem*() name, but since that was rejected, it's worth making up for in the docs.) |
|
|
| msg183975 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-03-11 16:50 |
| New changeset 8948dd77b095 by Benjamin Peterson in branch 'default': say defaultobj is returned (#17327) http://hg.python.org/cpython/rev/8948dd77b095 |
|
|
| msg183977 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-11 16:56 |
| ... and it's called "defaultobj", not "defautobj". |
|
|
| msg183978 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-03-11 17:17 |
| New changeset f6f39ebd3121 by Benjamin Peterson in branch 'default': fix spelling (#17327) http://hg.python.org/cpython/rev/f6f39ebd3121 |
|
|
| msg183980 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2013-03-11 17:28 |
| Thanks! |
|
|