Skip to content
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

Merged
merged 3 commits into from Sep 24, 2021
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Aug 22, 2021

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.


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.

converters = [int, float, int, float]
lines = [  # Assume that we already `str.split` them.
    ["1", "2.2", "3", "4.4"],
    ["5", "6.6", "7", "8.8"],
]
[[*map(operator.call, converters, words)] for words in lines]
# faster than the list comprehension
[[conv(word) for conv, word in zip(converters, words)] for words in lines]

Here the performance actually matters (with numpy, one may easily be loading millions of rows). A simple benchmark gives

$ ./python -mtimeit -s 'from operator import call; funcs = [int, float, float, int]; inputs = ["1", "2", "3", "4"]' -- '[*map(call, funcs, inputs)]'
500000 loops, best of 5: 761 nsec per loop
$ ./python -mtimeit -s 'from operator import call; funcs = [int, float, float, int]; inputs = ["1", "2", "3", "4"]' -- '[func(inp) for func, inp in zip(funcs, inputs)]'
200000 loops, best of 5: 1.07 usec per loop

i.e. map(operator.call, ...) gives a significant speedup.

https://bugs.python.org/issue44019

Lib/operator.py Outdated Show resolved Hide resolved
Lib/test/test_operator.py Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 23, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Sep 23, 2021
@anntzer
Copy link
Contributor Author

anntzer commented Sep 23, 2021

From my side, this is ready to go, afaict.

@merwok merwok requested review from mdickinson, vstinner and ambv Sep 23, 2021

The following operation works with callables:

.. function:: call(obj, *args, **kwargs)
Copy link
Member

@vstinner vstinner Sep 23, 2021

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.

Suggested change
.. function:: call(obj, *args, **kwargs)
.. function:: call(obj, /, *args, **kwargs)

Copy link
Contributor Author

@anntzer anntzer Sep 23, 2021

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
Copy link
Member

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)

Copy link
Member

@vstinner vstinner left a comment

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 :-)

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Sep 24, 2021
@anntzer
Copy link
Contributor Author

anntzer commented Sep 24, 2021

Apologies for the squash commit, I did not realize this was the policy here.

Copy link
Member

@mdickinson mdickinson left a comment

LGTM. This seems like a fairly natural addition to the operator module for me.

@mdickinson
Copy link
Member

mdickinson commented Sep 24, 2021

Do we need a "what's new" entry for this?

@anntzer
Copy link
Contributor Author

anntzer commented Sep 24, 2021

There's a NEWS.d entry, did you mean anything else?

@mdickinson
Copy link
Member

mdickinson commented Sep 24, 2021

@vstinner
Copy link
Member

vstinner commented Sep 24, 2021

Ah yes please, document the addition in What's New in Python 3.11.

@anntzer
Copy link
Contributor Author

anntzer commented Sep 24, 2021

Done.

@vstinner
Copy link
Member

vstinner commented Sep 24, 2021

Done.

I don't see any change on Doc/whatsnew/3.11.rst.

* 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`.)
Copy link
Member

@vstinner vstinner Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* 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`.)

Copy link
Contributor Author

@anntzer anntzer Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woopsie, thanks, fixed.

Copy link
Member

@vstinner vstinner left a comment

LGTM. I let another core dev merge this PR.

@mdickinson mdickinson merged commit 6587fc6 into python:main Sep 24, 2021
12 checks passed
@anntzer anntzer deleted the operator.call branch Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants