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

Expose meaningful keyword arguments for pow() #82418

Closed
rhettinger opened this issue Sep 20, 2019 · 28 comments
Closed

Expose meaningful keyword arguments for pow() #82418

rhettinger opened this issue Sep 20, 2019 · 28 comments
Assignees
Labels
3.8 only security fixes 3.9 only security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@rhettinger
Copy link
Contributor

BPO 38237
Nosy @rhettinger, @mdickinson, @ambv, @serhiy-storchaka, @ammaraskar, @pablogsal, @miss-islington, @lschoe
PRs
  • bpo-38237: Make pow's arguments have more descriptive names and be keyword passable #16302
  • [3.8] bpo-38237: Make pow's arguments have more descriptive names and be keyword passable (GH-16302) #16320
  • [3.8] bpo-38237: Shorter docstring (GH-16322) #16323
  • bpo-38237: Fix "versionchanged" for pow named arguments #19042
  • [3.8] bpo-38237: Fix "versionchanged" for pow named arguments (GH-19042) #19079
  • bpo-38237: Use divmod for positional arguments whatsnew example #19171
  • [3.8] bpo-38237: Use divmod for positional arguments whatsnew example (GH-19171) #19192
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/rhettinger'
    closed_at = <Date 2019-09-21.16:50:01.178>
    created_at = <Date 2019-09-20.17:21:42.262>
    labels = ['easy', '3.8', 'type-feature', 'library', '3.9']
    title = 'Expose meaningful keyword arguments for pow()'
    updated_at = <Date 2020-03-27.16:45:09.009>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2020-03-27.16:45:09.009>
    actor = 'miss-islington'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2019-09-21.16:50:01.178>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2019-09-20.17:21:42.262>
    creator = 'rhettinger'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38237
    keywords = ['patch', 'easy (C)']
    message_count = 28.0
    messages = ['352876', '352877', '352878', '352879', '352880', '352881', '352885', '352886', '352887', '352895', '352900', '352923', '352924', '352927', '352933', '352939', '352944', '352951', '352952', '364395', '364396', '364397', '364400', '364401', '364402', '364414', '365166', '365167']
    nosy_count = 8.0
    nosy_names = ['rhettinger', 'mark.dickinson', 'lukasz.langa', 'serhiy.storchaka', 'ammar2', 'pablogsal', 'miss-islington', 'lschoe']
    pr_nums = ['16302', '16320', '16323', '19042', '19079', '19171', '19192']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue38237'
    versions = ['Python 3.8', 'Python 3.9']

    @rhettinger
    Copy link
    Contributor Author

    Current signature:

        pow(x, y, z=None, /)

    Proposed signature:

        pow(base, exp, mod=None)

    Benefits:

    • Meaningful and self-explanatory parameters in tooltips

    • Optionally clearer calls for the three argument form:

         pow(2, 5, mod=4)
    • More usable with partial():
         squared = partial(pow, exp=2)

    @rhettinger rhettinger added 3.9 only security fixes stdlib Python modules in the Lib dir easy type-feature A feature request or enhancement labels Sep 20, 2019
    @ammaraskar
    Copy link
    Member

    Looks like a solid proposal, I especially like the clarity for the 3-argument call. Often beginners ask about why there's a third argument in pow especially when teaching RSA and number-theoretic stuff.

    Do you mind if I take this on Raymond?

    @ammaraskar
    Copy link
    Member

    Actually quick question, should a similar change be made for math.pow for consistency's sake?

    @ammaraskar
    Copy link
    Member

    I've made a PR, feel free to close it if you'd rather implement this yourself or this proposal won't be accepted :)

    @serhiy-storchaka
    Copy link
    Member

    You can use a lambda instead of partial:

        squared = lambda x: pow(x, 2)

    Proposed names look meaningful. But after adding support of keyword arguments please compare performance of the old and the new functions. I expect that the difference will be small, but we need to check.

    @ammaraskar
    Copy link
    Member

    Here's a little microbenchmark, let me know if there's anything specific you'd like to see:

    Before
    ======

    python -m pyperf timeit "from test.test_builtin import BuiltinTest; tst = BuiltinTest()" -- "tst.test_pow()"

    Mean +- std dev: 3.80 us +- 0.23 us

    python -m pyperf timeit "pow(23, 19, 3)"

    Mean +- std dev: 519 ns +- 12 ns

    After
    =====

    python -m pyperf timeit "from test.test_builtin import BuiltinTest; tst = BuiltinTest()" -- "tst.test_pow()"

    Mean +- std dev: 3.80 us +- 0.26 us

    python -m pyperf timeit "pow(23, 19, 3)"

    Mean +- std dev: 526 ns +- 18 ns

    @mdickinson
    Copy link
    Member

    The proposal sounds reasonable to me.

    should a similar change be made for math.pow for consistency's sake?

    I'd leave math.pow alone here.

    @serhiy-storchaka
    Copy link
    Member

    Thank you. Could you please test simpler examples like pow(2, 3)? Please use the --duplicate option.

    @serhiy-storchaka
    Copy link
    Member

    And pow(2.0, 3.0) please.

    @ammaraskar
    Copy link
    Member

    Before
    ======

    python -m pyperf timeit "pow(2, 3)" --duplicate 100000
    Mean +- std dev: 242 ns +- 19 ns

    python -m pyperf timeit "pow(2.0, 3.0)" --duplicate 100000
    Mean +- std dev: 197 ns +- 16 ns

    After
    =====

    python -m pyperf timeit "pow(2, 3)" --duplicate 100000
    Mean +- std dev: 243 ns +- 11 ns

    python -m pyperf timeit "pow(2.0, 3.0)" --duplicate 100000
    Mean +- std dev: 200 ns +- 14 ns

    @ammaraskar
    Copy link
    Member

    math.pow changes removed from PR

    @rhettinger rhettinger self-assigned this Sep 21, 2019
    @miss-islington
    Copy link
    Contributor

    New changeset 87d6cd3 by Miss Islington (bot) (Ammar Askar) in branch 'master':
    bpo-38237: Make pow's arguments have more descriptive names and be keyword passable (GH-16302)
    87d6cd3

    @rhettinger
    Copy link
    Contributor Author

    Thanks Ammar

    @serhiy-storchaka
    Copy link
    Member

    Thank you for your contribution Ammar! Nice work!

    @rhettinger
    Copy link
    Contributor Author

    New changeset 37bc935 by Raymond Hettinger (Miss Islington (bot)) in branch '3.8':
    bpo-38237: Let pow() support keyword arguments (GH-16302) (GH-16320)
    37bc935

    @serhiy-storchaka
    Copy link
    Member

    Isn't it a new feature? Isn't it too later to add it to 3.8?

    @rhettinger
    Copy link
    Contributor Author

    As noted in the checkin, this was backported with the release manager's assent.

    FWIW, pow() itself is an old feature, recently enhanced to support negative powers in a given modulus. When the enhancement went in, we should have done this as well.

    @rhettinger rhettinger added the 3.8 only security fixes label Sep 21, 2019
    @rhettinger
    Copy link
    Contributor Author

    New changeset 24231ca by Raymond Hettinger (Miss Islington (bot)) in branch '3.8':
    bpo-38237: Shorter docstring (GH-16322) (GH-16323)
    24231ca

    @serhiy-storchaka
    Copy link
    Member

    Thank you for the explanation Raymond and sorry for the disturb. My mistake, I had not noticed the release manager's assent.

    @lschoe
    Copy link
    Mannequin

    lschoe mannequin commented Mar 17, 2020

    There seems to be a slight mixup with the built-in pow() function in Python 3.8.2.

    Currently, under https://docs.python.org/3/library/functions.html#pow it says:

    Changed in version 3.9: Allow keyword arguments. Formerly, only positional arguments were supported.
    

    I think this should be into "Changed in version 3.8 ... ", as pow(3,4, mod=5) actually works in Python 3.8.2.

    The "What’s New In Python 3.8" also needs to be changed accordingly.
    In https://docs.python.org/3/whatsnew/3.8.html#positional-only-parameters it says:

    One use case for this notation is that it allows pure Python functions to fully emulate behaviors of existing C coded functions. For example, the built-in pow() function does not accept keyword arguments:
    	def pow(x, y, z=None, /):
    		"Emulate the built in pow() function"
    		r = x ** y
    		return r if z is None else r%z
    

    This example can simply be dropped now.

    @mdickinson
    Copy link
    Member

    This example can simply be dropped now.

    It could be, but it would be better to replace it with a different example that works. Any suggestions?

    @ammaraskar
    Copy link
    Member

    @serhiy-storchaka
    Copy link
    Member

    It is hard to find a builtin which could be easy and clearly implemented in Python (it means no use of dunder methods). Maybe sorted()?

    def sorted(iterable, /, *, key=None, reverse=False):
        """Emulate the built in sorted() function"""
        result = list(iterable)
        result.sort(key=key, reverse=reverse)
        return result

    Although I think that this use case is less important. The primary goal of the feature is mentioned at the end of the section -- easy way to implement functions which accept arbitrary keyword arguments.

    @serhiy-storchaka
    Copy link
    Member

    divmod() should be implemented via __divmod__ and __rdivmod__. And they should be looked up on the type, not instance. There is no easy way to express it in Python accurately. This would make the example too complex.

    @ammaraskar
    Copy link
    Member

    I don't think that matters. The example is supposed to just serve as an illustration, it doesn't need to encode the dunder dispatch semantics. The already existing example doesn't check for a __pow__.

    I'd picture it just as:

    return x//y, x%y

    @lschoe
    Copy link
    Mannequin

    lschoe mannequin commented Mar 17, 2020

    Maybe a use case in this direction: int(x, base=10).
    Because, if you type

        int(x='3', base=12)

    you get

    TypeError: 'x' is an invalid keyword argument for int()
    

    and x needs to be a positional-only to program this yourself.

    @pablogsal
    Copy link
    Member

    New changeset 5a58c52 by Ammar Askar in branch 'master':
    bpo-38237: Use divmod for positional arguments whatsnew example (GH-19171)
    5a58c52

    @miss-islington
    Copy link
    Contributor

    New changeset 9c5c497 by Miss Islington (bot) in branch '3.8':
    bpo-38237: Use divmod for positional arguments whatsnew example (GH-19171)
    9c5c497

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants