Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-44019: Implement operator.call().#27888
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
anntzer commented Aug 22, 2021 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This PR is stale because it has been open for 30 days with no activity. |
anntzer commented Sep 23, 2021
From my side, this is ready to go, afaict. |
Doc/library/operator.rst Outdated
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.
merwok commented Sep 23, 2021
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) |
vstinner left a comment
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 :-)
anntzer commented Sep 24, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Apologies for the squash commit, I did not realize this was the policy here. |
mdickinson left a comment
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.
mdickinson commented Sep 24, 2021
Do we need a "what's new" entry for this? |
anntzer commented Sep 24, 2021
There's a NEWS.d entry, did you mean anything else? |
mdickinson commented Sep 24, 2021
@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). |
vstinner commented Sep 24, 2021
Ah yes please, document the addition in What's New in Python 3.11. |
anntzer commented Sep 24, 2021
Done. |
vstinner commented Sep 24, 2021
I don't see any change on Doc/whatsnew/3.11.rst. |
anntzer commented Sep 24, 2021
| * 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.
vstinner left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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, **kwargsthen 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