-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-44019: Implement operator.call(). #27888
Conversation
This PR is stale because it has been open for 30 days with no activity. |
From my side, this is ready to go, afaict. |
Doc/library/operator.rst
Outdated
|
||
The following operation works with callables: | ||
|
||
.. function:: call(obj, *args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add / to the prototype.
.. function:: call(obj, *args, **kwargs) | |
.. function:: call(obj, /, *args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, thanks for noticing.
Having `operator.call(obj, arg)` mean `type(obj).__call__(obj, arg)` is consistent with the other dunder operators. The semantics with `*args, **kwargs` then follow naturally from the single-arg semantics.
b13e539
to
344751b
Compare
The user experience isn’t good for reviewers when history is rewritten in a PR, so please do regular commits and pushes for CPython (see https://devguide.python.org/pullrequest/#submitting) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The implementation is correct. I'm not convinced of the usefulness of this change, but the benchmark is interesting. I let other core dev merging the change :-)
Apologies for the squash commit, I did not realize this was the policy here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. This seems like a fairly natural addition to the operator
module for me.
Do we need a "what's new" entry for this? |
There's a NEWS.d entry, did you mean anything else? |
@anntzer: I meant the https://github.com/python/cpython/blob/main/Doc/whatsnew/3.11.rst file. I think it's worth a mention there (following the guidelines in https://devguide.python.org/committing/?highlight=what%27s%20new#updating-news-and-what-s-new-in-python). |
Ah yes please, document the addition in What's New in Python 3.11. |
Done. |
I don't see any change on Doc/whatsnew/3.11.rst. |
Doc/whatsnew/3.11.rst
Outdated
* A new function ``operator.call`` has been added, such that | ||
``operator.call(obj, *args, **kwargs) == obj(*args, **kwargs)``. | ||
(Contributed by Antony Lee in :issue:`44019`.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* A new function ``operator.call`` has been added, such that | |
``operator.call(obj, *args, **kwargs) == obj(*args, **kwargs)``. | |
(Contributed by Antony Lee in :issue:`44019`.) | |
* A new function ``operator.call`` has been added, such that | |
``operator.call(obj, *args, **kwargs) == obj(*args, **kwargs)``. | |
(Contributed by Antony Lee in :issue:`44019`.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woopsie, thanks, fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I let another core dev merge this PR.
Having
operator.call(obj, arg)
meantype(obj).__call__(obj, arg)
isconsistent with the other dunder operators. The semantics with
*args, **kwargs
then follow naturally from the single-arg semantics.I realize that I proposed different semantics in the original bug report, but that didn't even actually match to the proposed use case; the second proposal in the thread is the correct one. The semantics here work for at least two different use cases that I have seen:
Using concurrent.futures.Executor.map on a list of callables (
executor.map(operator.call, callables)
). In that case the performance doesn't actually matter much and a Python helper would in fact be fine.Loading a "typed" csv file (cf. numpy.loadtxt), where each column has a separate type, e.g.
Here the performance actually matters (with numpy, one may easily be loading millions of rows). A simple benchmark gives
i.e.
map(operator.call, ...)
gives a significant speedup.https://bugs.python.org/issue44019