See python-dev thread about my changeset 7efddbf1aa70:
https://mail.python.org/pipermail/python-dev/2016-November/146885.html

Serhiy asked me to revert it. It's now reverted by the revision 20bb8babc505. So, here is my change as a patch to review it.

See the thread for my rationale on argument names.

--

Uniformize argument names of "call" functions

* Callable object: callable, o, callable_object => func
* Object for method calls: o => obj
* Method name: name or nameid => method

Cleanup also the C code:

* Don't initialize variables to NULL if they are not used before their first
  assignement
* Add braces for readability

Independently of anything else, any changes made to the parameter names in the implementation should be reflected in the API docs: https://docs.python.org/3/c-api/object.html#c.PyCallable_Check

A style guide addition to PEP 7 would also be useful as a record of the preferred naming scheme.

That said, one of the problems you're up against inside the C code base is that we really do have some code evaluation APIs that only work with true Python functions, while a abstract APIs handle arbitrary callables. Reserving "func" for the former cases helps make it clear which is which.

By contrast, end user code (and even the standard library) can usually get away with saying "func" even when they mean "arbitrary callable" without causing confusion, as there's only the one Python-level call syntax.

So perhaps one way to start here would be to propose an addition to https://www.python.org/dev/peps/pep-0007/#naming-conventions specifically for dealing with callable parameters in APIs that covers parameters that are:

- arbitrary callables
- specifically a synchronous Python function
- method names (whether as const char *, PyObject *, or _Py_Identifier)

Serhiy: Can you please elaborate "Other changes looks not well justified too."?

Isn't this just a lot of churn for not a lot of benefit? I'm all for consistency, but you'll break patches on the bug tracker and externally maintained patches.

> * Callable object: callable, o, callable_object => func

The original names better describes the argument type in functions like PyObject_Call(). It works not just with functions, but with arbitrary callables. For example it is used with type objects for creating instances.

> * Method name: name or nameid => method

The parameter is not a *method object*, but a method *name*. You have to pass an object and a method names to call a method.

> Serhiy: Can you please elaborate "Other changes looks not well justified too."?

Renaming obj to arg0 looks questionable to me. I would rather rename func to method (this is a true unbound method, not method name as in PyObject_CallMethod()).

It would be better to uniformize parameter names in the documentation (and perhaps in headers) and left sources untouched unless we rewrite particular functions. Otherwise it looks as code churn. It would be nice to formalize naming rules in PEP 7.

See also issue18697.

So, what do you think of renaming argument like:

* Replace "callable_object" and "func" with "callable"
* Replace "method" with "name"
* Rename "o" to "obj" (ex: in "PyCallable_Check(PyObject *o)")

I prefer "callable_object" over "callable" because it's shorter. Some functions already use "callable" (and so others use "callable_object").


Eric: ""Isn't this just a lot of churn for not a lot of benefit? I'm all for consistency, but you'll break patches on the bug tracker and externally maintained patches."

I'm not aware of any pending patch on "call" functions like PyObject_Call. I think that I am the latest who modified these functions. I'm working on a new patch serie to add support for fast calls in tp_call. That's why I would prefer to uniformize argument names first, before rebasing my large patch.

I withdraw my concern. I was thinking it would affect the caller, but of course it won't. Apologies.

As long as the C API docs also get updated, +1 for:

* 'o' -> 'obj'
* 'callable_object' -> 'callable'
* 'func' -> 'callable' (when the function actually accepts an arbitrary callable)
* 'method' -> 'name' (assuming 'Method' appears in the function name)

Documenting those naming conventions in PEP 7 would also still be desirable.

rename.patch: Big patch to rename function parameters.

Added comments on Rietveld.

I think we can keep "func" instead of "callable" in the source code for decreasing the number of changes.

New changeset e6ad41a3f3bd by Victor Stinner in branch 'default':
Uniformize argument names of "call" functions
https://hg.python.org/cpython/rev/e6ad41a3f3bd

I pushed a change which is rename.patch with fixes on comments in abstract.h. It seems like comments were no more revelant or moved to random places. I moved many comments, rewrote some of them, etc. Some comments were wrong like "This is the equivalent of the Python expression: o.method(args)", it's "o.method(*args)" in fact. Serhiy catched this mistake, it should now be fixed.

I chose to push this large change, instead of having another round of review, because it's tricky to handle such large changes. I suggest to push following changes if you still see issues.

I also wanted to push it quickly because I have pending changes modifying the same parts of the code in my queue :-)

Thanks for reviews Serhiy! Very useful, as usual ;-)


Serhiy: "I think we can keep "func" instead of "callable" in the source code for decreasing the number of changes."

I prefer to "fix" it as well, the purpose of the issue is to uniformize everything: .c files, .h files and .rst documentation.

"Uniformize" isn't really an English word :)

Oh, you have mixed up comments! In headers descriptions are written after function declarations. You have moved some comments and now some descriptions are placed below the corresponding declaration, and some descriptions are placed above. Now it is not clear to what function every comment is related.

You should either revert your changes or change *all* headers by moving *all* descriptions above corresponding declarations. That would be very large patch, and I doubt it will be approved.

Please don't make such large changes without a review.

New changeset 69c7451c3ec1 by Victor Stinner in branch 'default':
Issue #28838: Fix weird indentation of abstract.h
https://hg.python.org/cpython/rev/69c7451c3ec1

I pushed another large patch to "fix" the indentation of abstract.h. I would prefer to also have this "reindent" change on review, but our reviewing tool doesn't support patch series :-(


Attached cleanup.patch reformats deeply the header file to use the same style for all comments, use the same style used in other Python headers. IMO it makes the header file more consistent with other header files and it makes the file more readable.


R. David Murray (r.david.murray):
> Uniformize" isn't really an English word :)

Oh, it's a frenglish word in this case :-) Good to know.


Serhiy Storchaka: "You have moved some comments and now some descriptions are placed below the corresponding declaration, and some descriptions are placed above. Now it is not clear to what function every comment is related."

It don't think that my change makes abstract.h worse, some comments were far from their declaration, and there is was no unique style for comments. It's a mess.


Serhiy: "You should either revert your changes or change *all* headers by moving *all* descriptions above corresponding declarations."

My plan is to rewrite completely abstact.h, this file annoys me since many months :-)


"That would be very large patch, and I doubt it will be approved."

Why not?


"Please don't make such large changes without a review."

You reviewed rename.patch, I only made tiny changes after this patch. As I explained, it's hard to work on such large patch.

> It don't think that my change makes abstract.h worse, some comments were far
> from their declaration, and there is was no unique style for comments. It's
> a mess.

Seems most of comments that did not follow common style are your comments to 
"fast call" functions. It would be better to fix only that comments.

> "That would be very large patch, and I doubt it will be approved."
> Why not?

Because it breaks "hg annotation" and makes harder researching the history of 
the file. That harms the workflow of other developers. Ask on Python-Dev before 
making such larger and questionable changes.

> You reviewed rename.patch, I only made tiny changes after this patch.

Reviewed patch did not contain committed changes. That changes are not tiny.

Serhiy:
> Seems most of comments that did not follow common style are your comments to  "fast call" functions. It would be better to fix only that comments.

I would like to modify abstract.h because its weird style annoy me for years. I would like to make the header consistent with all other headers.

I checked unicodeobject.h, code.h, frameobject.h, floatobject.h, etc.: comments are always before declaration.

About "fast calls" in abstract.h, sorry, it's my fault. When I added new code, I used the same style than all other files. I didn't notice that abstract.h has a special style. That's why I would like to make it consistent again.


Serhiy:
> Reviewed patch did not contain committed changes.

You wrote on the review that a comment is misplaced: "The comment is related to PyObject_Call() declared 85 lines above."

That's why I took the opportunity of a refactoring change to fix comments.


Serhiy:
> That changes are not tiny.

Between rename.patch and the pushed change e6ad41a3f3bd, I only moved 5 comments: see attached rename_pushed.patch which is the diff between the two patches.

For me, it was obvious that the comment must be before the declaration, since it is the style used in all other Python headers. Sorry, I didn't expect that you didn't want to change that. I misunderstood your comment on the review.

cleanup-2.patch: Rebase cleanup.patch.

This patch is the last part of my work to cleanup Include/abstract.h to make it more consistent with other header files.

New changeset 654ec6ed3225 by Victor Stinner in branch 'default':
Issue #28838: Cleanup abstract.h
https://hg.python.org/cpython/rev/654ec6ed3225

Victor, it would be worth to discuss this change on Python-Dev.

I pushed cleanup-2.patch (after a minor rebase). Sorry for breaking "hg annotation" on Include/abstract.h: I hope that it will be less annoying that the giant "replace tabs with space" made by Antoine Pitrou a few years ago which modified almost all .c and .h files (my change only changes a single file, abstract.h). If you have large pending patch series which modify this file, ping me, I will help you to rebase them.

Sorry, I didn't expect this issue to be so painful. I hope that abstract.h is now more readable, easier to maintain (no more surprising indentation!), and have better comments. I now started to cleanup the documentation of functions of the "PyObject_Call() family": see my first issue #28977. I think that we should then synchronize .rst doc and .h comments to make them consistent again (and maybe update the doc).

Serhiy: "Victor, it would be worth to discuss this change on Python-Dev."

Done: https://mail.python.org/pipermail/python-dev/2016-December/146999.html

Thanks Victor.

I consider that the cleanup itself is done, so I close the issue. For further enhancements, I will open new issues.

New changeset 3ab0a6692e25 by Victor Stinner in branch 'default':
abstract.h: remove long outdated comment
https://hg.python.org/cpython/rev/3ab0a6692e25

Since no one on Python-Dev has objections, I'm fine with this too. I was opposed not so much to the changes themselves, as to making large changes to one of basic files that could affect everybody without wider discussion. Excuse me if I looked unfriendly.

Serhiy Storchaka added the comment:
> Since no one on Python-Dev has objections, I'm fine with this too. I was opposed not so much to the changes themselves, as to making large changes to one of basic files that could affect everybody without wider discussion. Excuse me if I looked unfriendly.

No problem. Well, it's just reformating. It doesn't change the API for
example. It should only impact people maintaining large external
patches on Python 3.6. I'm not aware of any pending patch modifying
abstract.h. And as I wrote, if there is such patch, contact me, I will
help to rebase your change.