classification
Title: Argument Clinic: convert dict methods
Type: performance Stage: resolved
Components: Argument Clinic Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.snow, haypo, inada.naoki, larry, martin.panter, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-01-18 16:54 by haypo, last changed 2017-05-03 16:41 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
dict.patch haypo, 2017-01-18 17:13
dict-docstrings.patch serhiy.storchaka, 2017-01-21 22:01 review
Messages (19)
msg285742 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-01-18 16:54
The dict type has multiple methods which use METH_VARARGS or METH_VARARGS|METH_KEYWORDS calling convention, whereras there is no a new faster METH_FASTCALL calling convention which avoids temporary tuple/dict to pass arguments.
msg285746 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-01-18 17:13
dict.patch converts two methods to Argument Clinic:

* get()
* setdefault()

pop() requires the issue #29299, its signature must not show any default value for the second parameter.

dict.update() is special, so I created a dedicated issue: issue #29312.
msg285765 - (view) Author: INADA Naoki (inada.naoki) * (Python committer) Date: 2017-01-19 00:57
LGTM
msg285775 - (view) Author: Roundup Robot (python-dev) Date: 2017-01-19 11:41
New changeset 00c63ee66e0c by Victor Stinner in branch 'default':
dict.get() and dict.setdefault() now use AC
https://hg.python.org/cpython/rev/00c63ee66e0c
msg285812 - (view) Author: Roundup Robot (python-dev) Date: 2017-01-19 17:00
New changeset fe1d83fe29d6 by Serhiy Storchaka in branch 'default':
Issue #29311: Argument Clinic generates reasonable name for the parameter "default".
https://hg.python.org/cpython/rev/fe1d83fe29d6
msg285813 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-19 17:02
There are same problems with docstrings as in OrderedDict. The name "D" is not defined.
msg285818 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-01-19 17:31
> Argument Clinic generates reasonable name for the parameter "default".
> -    default as failobj: object = None
> +    default: object = None

Thanks, that's a better name :-)

FYI I wanted to limit changes when converting to AC, the change is already big enough and hard to review.
msg285819 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-01-19 17:33
> There are same problems with docstrings as in OrderedDict. The name "D" is not defined.

I copied the old docstring to AC.

Python 3.6 doc:
---
get(...)
    D.get(k[,d]) -> D[k] if k in D, else d.  d defaults to None.
---

D was already implicitly "self".


Python 3.7 (new) doc:
---
get(self, key, default=None, /)
    D.get(key[, default]) -> D[key] if key in D, else default.
---

What do you propose? Use self?
msg285821 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-19 17:50
There was a context in old docstrings removed in new docstrings.

It is not always possible just to copy docstrings in Argument Clinic. Sometimes rewriting docstrings is the hardest part of converting to Argument Clinic.

I suggest either restore the context that defines D, or better totally rewrite docstrings, getting the wording from the documentation.
msg285845 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-01-19 21:10
D.get(key[, default]) -> D[key] if key in D, else default.

There is no big problem with that. D is defined at the start. The only thing I would have suggested is avoid using square brackets to mean two things in the one expression. Since it is no longer the signature, calling with both parameters should be okay:

'''
get($self, key, default=None, /)
--
D.get(key, default) -> D[key] if key in D, else default.
'''

However the other method no longer defines D:

'''
setdefault($self, key, default=None, /)
--
D.get(key,default), also set D[key]=default if key not in D.
'''

You could restore the initial text as “D.setdefault(key,default) ->”, or maybe rewrite it like

“Like get(), but also insert the default value if it is missing.”
msg285968 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-21 22:01
Proposed patch fixes/improves docstrings of dict and OrderedDict methods.
msg285981 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-01-22 02:35
Patch looks good, apart from one little thing (see review)
msg286221 - (view) Author: Roundup Robot (python-dev) Date: 2017-01-24 22:30
New changeset ffc0840762e4 by Serhiy Storchaka in branch 'default':
Issues #29311, #29289: Fixed and improved docstrings for dict and OrderedDict
https://hg.python.org/cpython/rev/ffc0840762e4
msg286222 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-24 22:31
Thank you for your review Martin.
msg286644 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-02-01 16:11
Thank you very much for the docstring enhancement Serhiy! I like your new docstrings.

It seems like all known issues are now fixed, so I close the issue. Thanks Naoki and Martin too for the reviews.
msg286912 - (view) Author: Roundup Robot (python-dev) Date: 2017-02-04 06:05
New changeset 222b9392a83b by Serhiy Storchaka in branch 'default':
Issue #29311: Regenerate Argument Clinic.
https://hg.python.org/cpython/rev/222b9392a83b
msg286918 - (view) Author: Roundup Robot (python-dev) Date: 2017-02-04 07:00
New changeset 7baf0a0b90da5bb0b1ed5d27374f5a6b1c7b5dae by Serhiy Storchaka in branch 'master':
Issue #29311: Regenerate Argument Clinic.
https://github.com/python/cpython/commit/7baf0a0b90da5bb0b1ed5d27374f5a6b1c7b5dae
msg292910 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-05-03 16:05
IMO, you've made the docstrings for pure Python versions of collections.OrderedDict worse.  The previous docstrings were clearer.

Please note, there is a high bar for altering docstrings that have worked well (been deployed with success) for over 15 years (they were modeled on the dict object docstrings).  You're undoing some of the work originally done by Guido van Rossum and Tim Peters.

Argument clinic was accepted on the premise that it would add useful information, not that it would entail a wholesale rewrite of every docstring.
msg292922 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-05-03 16:41
> IMO, you've made the docstrings for pure Python versions of collections.OrderedDict worse.  The previous docstrings were clearer.

Can you please elaborate? Please reopen the issue or open a new one if you consider that it's worth it (this issue is currently closed).
History
Date User Action Args
2017-05-03 16:41:25hayposetmessages: + msg292922
2017-05-03 16:05:14rhettingersetmessages: + msg292910
2017-02-04 07:00:34python-devsetmessages: + msg286918
2017-02-04 06:05:31python-devsetmessages: + msg286912
2017-02-01 16:11:52hayposetstatus: open -> closed
resolution: fixed
messages: + msg286644
2017-01-24 22:31:46serhiy.storchakasetmessages: + msg286222
2017-01-24 22:30:33python-devsetmessages: + msg286221
2017-01-22 02:35:29martin.pantersetmessages: + msg285981
2017-01-21 22:01:17serhiy.storchakasetfiles: + dict-docstrings.patch

nosy: + rhettinger, eric.snow
messages: + msg285968

stage: resolved
2017-01-19 21:10:46martin.pantersetnosy: + martin.panter
messages: + msg285845
2017-01-19 17:50:42serhiy.storchakasetmessages: + msg285821
2017-01-19 17:33:32hayposetmessages: + msg285819
2017-01-19 17:31:17hayposetmessages: + msg285818
2017-01-19 17:02:26serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg285813
2017-01-19 17:00:59python-devsetmessages: + msg285812
2017-01-19 11:41:13python-devsetnosy: + python-dev
messages: + msg285775
2017-01-19 00:57:26inada.naokisetmessages: + msg285765
2017-01-18 17:13:20hayposetfiles: + dict.patch
keywords: + patch
messages: + msg285746
2017-01-18 16:54:10haypocreate