classification
Title: Document PyObject_CallFunction() special case more explicitly
Type: Stage:
Components: Documentation, Interpreter Core Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: docs@python, haypo, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-12-15 08:36 by haypo, last changed 2016-12-16 08:49 by serhiy.storchaka.

Files
File name Uploaded Description Edit
call_doc.patch haypo, 2016-12-15 08:36 review
Messages (5)
msg283256 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-12-15 08:36
The PyObject_CallFunction() function has a special case if the format string is "O". See my email thread on python-dev:
https://mail.python.org/pipermail/python-dev/2016-December/146919.html

Serhiy wrote: "It is documented for Py_BuildValue(), and the documentation of PyObject_CallFunction() refers to Py_BuildValue()." which is true, but I would prefer to be more explicit to avoid bad surprises (see issues #21209 and #28920).

Attached patch modifies PyObject_CallFunction() doc to mention the "O" format special case in the .rst doc and the .h comment.

The documentation of PyObject_CallMethod() and _PyObject_CallMethodId() should also be modified, but I would prefer to wait for a first review on the added text before modifying all functions.

I proposed to only modify Python 3.7 because Include/abstract.h was a in a bad shape: I rewrote this file to make it easier to maintain, but only in Python 3.7, see issue #28838.
msg283370 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-16 07:21
"O" is not the only special case. "S" and "N" unpack a tuple argument too. "O&" converter should return a tuple, otherwise it is an error. All other format codes are illegal for single argument.

I would just deprecate this feature (in PyObject_CallFunction, not in Py_BuildValue). The behavior of PyObject_CallFunction with a single argument can be made more consistent and useful.
msg283390 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-12-16 08:07
Serhiy: "O" is not the only special case. "S" and "N" unpack a tuple argument too. "O&" converter should return a tuple, otherwise it is an error. All other format codes are illegal for single argument.

Oh, I didn't know. I should update my documentation.


Serhiy: I would just deprecate this feature (in PyObject_CallFunction, not in Py_BuildValue). The behavior of PyObject_CallFunction with a single argument can be made more consistent and useful.

PyObject_CallFunction(func, "O", arg) should call func(arg). I don't want the "O" format (and "S" and "N").

What do you mean exactly by deprecating the feature? Emit a warning if and only if te format string is "O" (or "S" or "N") and Py_BuildValue() returns a tuple?

I guess that PyObject_CallFunction(func, "(O)", arg) doesn't need to be modified, it already always call func(arg), even if arg is a tuple.

I am also tempted to fix PyObject_CallFunction() (and similar functions) rather than documenting the special case, but I dislike the idea of a deprecation.

Let's say that calling PyObject_CallFunction(func, "O", arg) where arg is a tuple would emit a DeprecationWarning and call func(*arg) in Python 3.7, but call func(arg) with no warning in Python 3.8. I dislike this path because developers would try to make the warning quiet in Python 3.7, for example use "(O)" format string, which is less obvious and looks like a hack to me.


More and more applications use Python bleeding edge (the development branch, default), and more and more developers quickly test their application on the new Python stable release. Maybe we can "simply" fix the behaviour of PyObject_CallFunction() with no transition period:

* Python 3.6: PyObject_CallFunction(func, "O", arg) calls func(arg), or func(*arg) if arg is a tuple
* Python 3.7: PyObject_CallFunction(func, "O", arg) always calls func(arg)

The special case was never documented. In my experience, almost no developer rely on this feature. Most developers don't expect the feature and so write code which doesn't work with tuple arguments.
msg283391 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-12-16 08:14
If the behaviour of PyObject_CallFunction(func, "O", arg) is changed, an application relying on the current behaviour would have to replace:

   res = PyObject_CallFunction(func, "O", arg);

with:

   /* code working on any Python version */
   if (PyTuple_Check(arg))
       res = PyObject_Call(func, arg, NULL);
   else
       res = PyObject_CallFunction(func, "O", arg);
msg283393 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-16 08:49
> All other format codes are illegal for single argument.

I was wrong. PyObject_CallFunction() handles correctly a single non-tuple 
argument. The problem is only with tuple result of Py_BuildValue().

> What do you mean exactly by deprecating the feature? Emit a warning if and
> only if te format string is "O" (or "S" or "N") and Py_BuildValue() returns
> a tuple?

Emit a FutureWarning if the format string contains the single format code and 
Py_BuildValue() returns a tuple. In 3.8 or 3.9 the behavior will be changed.

> I dislike this path
> because developers would try to make the warning quiet in Python 3.7, for
> example use "(O)" format string, which is less obvious and looks like a
> hack to me.

PyObject_CallFunctionObjArgs(func, arg, NULL) seems obvious and perhaps even 
more efficient in all versions.

> More and more applications use Python bleeding edge (the development branch,
> default), and more and more developers quickly test their application on
> the new Python stable release. Maybe we can "simply" fix the behaviour of
> PyObject_CallFunction() with no transition period:

Ask on Python-Dev. I afraid this is too dangerous. We should have at least one 
release with a FutureWarning.

> Most developers don't expect the feature and so write
> code which doesn't work with tuple arguments.

That is why we should add a FutureWarning. If the code doesn't fail by 
accident, but just works incorrectly, this can be unnoticed. FutureWarning 
could help to fix possible bugs in older Python versions.
History
Date User Action Args
2016-12-16 08:49:50serhiy.storchakasetmessages: + msg283393
2016-12-16 08:14:52hayposetmessages: + msg283391
2016-12-16 08:07:54hayposetmessages: + msg283390
2016-12-16 07:21:19serhiy.storchakasetmessages: + msg283370
2016-12-15 08:36:01haypocreate