[issue29311] Argument Clinic: convert dict methods

[STINNER Victor]

STINNER Victor added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Raymond Hettinger]

Raymond Hettinger added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Roundup Robot]

Roundup Robot added the comment:


New changeset 7baf0a0b90da5bb0b1ed5d27374f5a6b1c7b5dae by Serhiy Storchaka in 
branch 'master':
Issue #29311: Regenerate Argument Clinic.
https://github.com/python/cpython/commit/7baf0a0b90da5bb0b1ed5d27374f5a6b1c7b5dae


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Roundup Robot]

Roundup Robot added the comment:

New changeset 222b9392a83b by Serhiy Storchaka in branch 'default':
Issue #29311: Regenerate Argument Clinic.
https://hg.python.org/cpython/rev/222b9392a83b

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[STINNER Victor]

STINNER Victor added the comment:

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.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your review Martin.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Roundup Robot]

Roundup Robot added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Martin Panter]

Martin Panter added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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

--
nosy: +eric.snow, rhettinger
stage:  -> resolved
Added file: http://bugs.python.org/file46374/dict-docstrings.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Martin Panter]

Martin Panter added the comment:

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

--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[STINNER Victor]

STINNER Victor added the comment:

> 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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[STINNER Victor]

STINNER Victor added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Roundup Robot]

Roundup Robot added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[Roundup Robot]

Roundup Robot added the comment:

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

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[INADA Naoki]

INADA Naoki added the comment:

LGTM

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[STINNER Victor]

STINNER Victor added the comment:

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.

--
keywords: +patch
Added file: http://bugs.python.org/file46331/dict.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29311] Argument Clinic: convert dict methods

[STINNER Victor]

New submission from STINNER Victor:

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.

--
components: Argument Clinic
messages: 285742
nosy: haypo, inada.naoki, larry
priority: normal
severity: normal
status: open
title: Argument Clinic: convert dict methods
type: performance
versions: Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com