Argument Clinic: convert dict methods

[vstinner]

BPO	29311
Nosy	@rhettinger, @vstinner, @larryhastings, @methane, @ericsnowcurrently, @vadmium, @serhiy-storchaka
Files	dict.patch dict-docstrings.patch

^{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 = None
closed_at = <Date 2017-02-01.16:11:52.450>
created_at = <Date 2017-01-18.16:54:10.235>
labels = ['3.7', 'expert-argument-clinic', 'performance']
title = 'Argument Clinic: convert dict methods'
updated_at = <Date 2017-05-03.16:41:25.389>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2017-05-03.16:41:25.389>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-02-01.16:11:52.450>
closer = 'vstinner'
components = ['Argument Clinic']
creation = <Date 2017-01-18.16:54:10.235>
creator = 'vstinner'
dependencies = []
files = ['46331', '46374']
hgrepos = []
issue_num = 29311
keywords = ['patch']
message_count = 19.0
messages = ['285742', '285746', '285765', '285775', '285812', '285813', '285818', '285819', '285821', '285845', '285968', '285981', '286221', '286222', '286644', '286912', '286918', '292910', '292922']
nosy_count = 8.0
nosy_names = ['rhettinger', 'vstinner', 'larry', 'methane', 'python-dev', 'eric.snow', 'martin.panter', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue29311'
versions = ['Python 3.7']

[vstinner]

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.

[vstinner]

dict.patch converts two methods to Argument Clinic:

• get()
• setdefault()

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

dict.update() is special, so I created a dedicated issue: issue bpo-29312.

[methane]

LGTM

[python-dev]

New changeset 00c63ee66e0c by Victor Stinner in branch 'default':
dict.get() and dict.setdefault() now use AC
https://hg.python.org/cpython/rev/00c63ee66e0c

[python-dev]

New changeset fe1d83fe29d6 by Serhiy Storchaka in branch 'default':
Issue bpo-29311: Argument Clinic generates reasonable name for the parameter "default".
https://hg.python.org/cpython/rev/fe1d83fe29d6

[serhiy-storchaka]

There are same problems with docstrings as in OrderedDict. The name "D" is not defined.

[vstinner]

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.

[vstinner]

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?

[serhiy-storchaka]

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.

[vadmium]

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.”

[serhiy-storchaka]

Proposed patch fixes/improves docstrings of dict and OrderedDict methods.

[vadmium]

Patch looks good, apart from one little thing (see review)