classification
Title: Error in Python 3 docs for PyMethodDef
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: asvetlov, barry, belopolsky, berker.peksag, bkabrda, cimarron, docs@python, jcea, martin.panter, md5i, python-dev, sandro.tosi, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2012-08-14 19:02 by sandro.tosi, last changed 2017-02-22 23:07 by barry. This issue is now closed.

Files
File name Uploaded Description Edit
issue15657.diff berker.peksag, 2015-04-22 07:12 review
issue15657_36.diff berker.peksag, 2016-06-12 13:54 review
Messages (14)
msg168226 - (view) Author: Sandro Tosi (sandro.tosi) * (Python committer) Date: 2012-08-14 19:02
Hello,
it has been reported at http://mail.python.org/pipermail/docs/2012-April/008215.html but given it raises some question whether it's a bug in the doc or in the code, i'd rather report the issue here and hear what other think:

>>>
In the Python 3.2.2 documentation (and earlier Python 3 versions), in
the Python/C API Reference Manual, chapter Object Implementation
Support, the documentation for PyMethodDef says:

  The ml_flags field is a bitfield which can include the following
  flags. The individual flags indicate either a calling convention or a
  binding convention. Of the calling convention flags, only METH_VARARGS
  and METH_KEYWORDS can be combined (but note that METH_KEYWORDS alone
  is equivalent to METH_VARARGS | METH_KEYWORDS).

The bit in the parenthetical is incorrect.  If you take a look at
PyCFunction_Call in Objects/methodobject.c, you will find a switch for
METH_VARARGS | METH_KEYWORDS, but no switch for METH_KEYWORDS.  Hence,
using METH_KEYWORDS will land you with a SystemError that complains
about METH_OLDARGS.

This is either a bug in the documentation, or a bug in Python.  In this
case, since the code has persisted through three major revisions of
Python 3, I suggest that the bug _is_ in the documentation (whether it
should be or not), since changing the code for this at this late date
means a programmer has to use METH_VARARGS | METH_KEYWORDS anyway for
compatibility.

It may be work pointing out specifically in the documentation that just
using METH_KEYWORDS will not work.
<<<
msg168463 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-17 18:41
I think we can change PyCFunction_Call to accept METH_KEYWORDS as alias for METH_VARARGS | METH_KEYWORDS.
It cannot make incompatibility with existing code base.
msg186676 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2013-04-12 20:26
I just ran into this too.
msg186679 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2013-04-12 20:31
We should fix the code for 3.2 through 3.4, but change the docs for 3.2 and 3.3 to remove the parenthetical note.  For 3.4 we can leave the parenthetical note but say this is new in 3.4 (or maybe, that it doesn't actually work in some versions < 3.4).  I agree that for code to work consistently across all Python 3.2 and 3.3 microversions, extension code is going to have to include both flags anyway.
msg211126 - (view) Author: Cimarron Mittelsteadt (cimarron) Date: 2014-02-13 03:44
Appears to be a duplicate of issue 11587 but better explanation here
msg241780 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-04-22 07:12
Here is a patch for Python 3.5.
msg247285 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-07-24 15:32
In 3.5 it would be better to make METH_KEYWORDS == METH_VARARGS | METH_KEYWORDS.

Current definition:

#define METH_VARARGS  0x0001
#define METH_KEYWORDS 0x0002

Should be:

#define METH_VARARGS  0x0001
#define METH_KEYWORDS 0x0003

But it can't be applied in maintained releases. In 3.4 and 2.7 we should add explicit test as in the patch or change the documentation.

If fix the code rather than documentation in 3.4 and 2.7, then the versionchanged directive in 3.5 shouldn't be added.
msg268091 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-06-10 05:56
I'm going to delete

    (but note that :const:`METH_KEYWORDS` alone is equivalent to ``METH_VARARGS | METH_KEYWORDS``)

from Doc/c-api/structures.rst in 3.5.

Then change the value of METH_KEYWORDS from 0x0002 to 0x0003 in 3.6.

Thanks for the review Serhiy.
msg268379 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-12 13:35
New changeset 367b3f41710a by Berker Peksag in branch '3.5':
Issue #15657: Delete incorrect statement from PyMethodDef documentation
https://hg.python.org/cpython/rev/367b3f41710a

New changeset 7fa4986d8218 by Berker Peksag in branch 'default':
Issue #15657: Null merge from 3.5
https://hg.python.org/cpython/rev/7fa4986d8218
msg268380 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-12 13:37
New changeset f520ae3b537b by Berker Peksag in branch '2.7':
Issue #15657: Delete incorrect statement from PyMethodDef documentation
https://hg.python.org/cpython/rev/f520ae3b537b
msg268382 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-06-12 13:54
> Then change the value of METH_KEYWORDS from 0x0002 to 0x0003 in 3.6.

Here is a patch.
msg285100 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-01-10 08:48
.
The documentation did not get merged properly into 3.6+. And even in 3.5, under METH_KEYWORDS, I propose to change “The flag is typically combined with METH_VARARGS” to “The flag must be combined . . .”.

The remaining issue15657_36.diff patch looks out of date. There is a _PyCFunction_FastCallDict() function that appears to also need adjusting.

But instead of changing the value of METH_KEYWORDS, wouldn’t it be safer to just accept METH_KEYWORDS (= 2) on its own as a valid value? This is what Python 2 does. Essentially, revert the first hunk of

https://hg.python.org/cpython/diff/b7bfa780a882/Objects/methodobject.c

but without METH_OLDARGS, whose value was zero anyway.

BTW, the statement did not need to be removed in Python 2, but IMO it’s fine now without the statment. The statement was added in revision 1564c6839e6b.
msg285218 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-01-11 12:16
New changeset d7d2d24003f5 by Martin Panter in branch '3.5':
Issue #15657: METH_KEYWORDS cannot be used alone in Python 3
https://hg.python.org/cpython/rev/d7d2d24003f5

New changeset c140e72492a4 by Martin Panter in branch '3.6':
Issue #15657: Delete incorrect statement from PyMethodDef documentation
https://hg.python.org/cpython/rev/c140e72492a4

New changeset 021fd2ff7ca4 by Martin Panter in branch '3.6':
Issue #15657: Merge other doc fix from 3.5
https://hg.python.org/cpython/rev/021fd2ff7ca4

New changeset 1058e151049a by Martin Panter in branch 'default':
Issue #15657: Merge METH_KEYWORDS doc from 3.6
https://hg.python.org/cpython/rev/1058e151049a
msg288390 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2017-02-22 23:07
I think this bug has been fixed.
History
Date User Action Args
2017-02-22 23:07:11barrysetstatus: open -> closed
resolution: fixed
messages: + msg288390

stage: patch review -> resolved
2017-01-11 12:16:00python-devsetmessages: + msg285218
2017-01-10 08:48:40martin.pantersetnosy: + martin.panter

messages: + msg285100
versions: + Python 3.7
2016-06-12 13:54:40berker.peksagsetfiles: + issue15657_36.diff

messages: + msg268382
2016-06-12 13:37:56python-devsetmessages: + msg268380
2016-06-12 13:35:31python-devsetnosy: + python-dev
messages: + msg268379
2016-06-10 05:56:23berker.peksagsetmessages: + msg268091
versions: + Python 3.6, - Python 3.4
2016-06-09 20:25:06belopolskysetnosy: + belopolsky
2015-07-24 15:32:16serhiy.storchakasetmessages: + msg247285
2015-04-22 07:13:28berker.peksaglinkissue11587 superseder
2015-04-22 07:12:25berker.peksagsetfiles: + issue15657.diff

type: behavior
components: + Interpreter Core, - Documentation
versions: + Python 3.5, - Python 3.2, Python 3.3
keywords: + patch
nosy: + berker.peksag, serhiy.storchaka

messages: + msg241780
2014-02-13 03:44:30cimarronsetnosy: + cimarron
messages: + msg211126
2013-11-29 14:19:07bkabrdasetnosy: + bkabrda
2013-04-12 20:31:58barrysetmessages: + msg186679
versions: + Python 3.4
2013-04-12 20:26:08barrysetnosy: + barry
messages: + msg186676
2012-08-17 18:41:35asvetlovsetmessages: + msg168463
2012-08-17 16:39:29asvetlovsetnosy: + asvetlov
2012-08-15 02:51:26jceasetnosy: + jcea
2012-08-14 19:42:18md5isetnosy: + md5i
2012-08-14 19:02:45sandro.tosicreate