FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-125957: sync argument naming in sphinx docs of the cmath and help() by skirpichev · Pull Request #125958 · python/cpython · GitHub

/ cpython Public

gh-125957: sync argument naming in sphinx docs of the cmath and help() - #125958

Merged
AA-Turner merged 5 commits into
python:mainfrom
skirpichev:fix-cmath-sig-125957
Apr 1, 2025
ConversationCommits (5)ChecksFiles changed
Merged

gh-125957: sync argument naming in sphinx docs of the cmath and help()#125958
AA-Turner merged 5 commits into
python:mainfrom
skirpichev:fix-cmath-sig-125957

Conversation

skirpichev commented Oct 25, 2024 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

pythongh-125957: sync argument naming in sphinx docs of the cmath and…
769c65f 
… help()
+ allow cmath.sin(z=1) syntax
f34de6a 
In the main:
$ ./python -m timeit -r11 -s 'from cmath import sin;z=1j' 'sin(z)'
1000000 loops, best of 11: 312 nsec per loop

With patch:
$ ./python -m timeit -r11 -s 'from cmath import sin;z=1j' 'sin(z)'
1000000 loops, best of 11: 330 nsec per loop
bedevere-app Bot mentioned this pull request Oct 25, 2024
Closed

skirpichev commented Oct 25, 2024

Copy link
Copy Markdown
Member Author

The second commits allows sin(z=1) syntax. This has some overhead:

In the main:
$ ./python -m timeit -r11 -s 'from cmath import sin;z=1j' 'sin(z)'
1000000 loops, best of 11: 312 nsec per loop

With patch:
$ ./python -m timeit -r11 -s 'from cmath import sin;z=1j' 'sin(z)'
1000000 loops, best of 11: 330 nsec per loop

Alternative solution: include slashed in the sphinx docs to reflect the actual function signature.

rruuaanng reviewed Oct 25, 2024
View reviewed changes
Comment thread Modules/clinic/cmathmodule.c.h Outdated
rruuaanng mentioned this pull request Oct 25, 2024
Closed
picnixz reviewed Feb 17, 2025
View reviewed changes
Comment thread Doc/library/cmath.rst Outdated
Comment thread Doc/library/cmath.rst Outdated
Apply suggestions from code review
ec3754e 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

picnixz commented Feb 17, 2025

Copy link
Copy Markdown
Member

I'm going offline until Friday so I won't be able to merge it (@vstinner asked me to wait for one other core dev approval before merging so if someone accepts it, they can either merge it or accept the PR and I'll merge it on Friday)

vstinner approved these changes Feb 17, 2025
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

PR title:

gh-125957: sync argument naming in sphinx docs of the cmath and help() #125958

It might be interesting to add the / marker for positional-only parameters.

Example:

>>> import cmath; help(cmath.phase)
phase(z, /)
    Return argument, also known as the phase angle, of a complex.

skirpichev commented Feb 18, 2025

Copy link
Copy Markdown
Member Author

It might be interesting to add the / marker for positional-only parameters.

I did this once: #101120

This thread discuss an alternative proposal (allow keyword arguments): https://discuss.python.org/t/81003/ See some benchmarks above: #125958 (comment)

skirpichev added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Mar 30, 2025
This was referenced Mar 30, 2025
Closed
Merged
AA-Turner merged commit 0a3eb88 into python:main Apr 1, 2025

miss-islington-app Bot commented Apr 1, 2025

Copy link
Copy Markdown

Thanks @skirpichev for the PR, and @AA-Turner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 1, 2025
pythongh-125957: sync argument naming in sphinx docs of the cmath and…
eb05118 
… help() (pythonGH-125958)

(cherry picked from commit 0a3eb88)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

bedevere-app Bot commented Apr 1, 2025

Copy link
Copy Markdown

GH-131962 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Apr 1, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 1, 2025
pythongh-125957: sync argument naming in sphinx docs of the cmath and…
58dbb66 
… help() (pythonGH-125958)

(cherry picked from commit 0a3eb88)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

bedevere-app Bot commented Apr 1, 2025

Copy link
Copy Markdown

GH-131963 is a backport of this pull request to the 3.12 branch.

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Apr 1, 2025
AA-Turner pushed a commit that referenced this pull request Apr 1, 2025
[3.13] gh-125957: sync argument naming in sphinx docs of the cmath an…
2745984 
…d help() (GH-125958) (#131962)

gh-125957: sync argument naming in sphinx docs of the cmath and help() (GH-125958)
(cherry picked from commit 0a3eb88)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
AA-Turner pushed a commit that referenced this pull request Apr 1, 2025
[3.12] gh-125957: sync argument naming in sphinx docs of the cmath an…
ce85eb5 
…d help() (GH-125958) (#131963)

gh-125957: sync argument naming in sphinx docs of the cmath and help() (GH-125958)
(cherry picked from commit 0a3eb88)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
skirpichev deleted the fix-cmath-sig-125957 branch April 1, 2025 07:24
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner approved these changes
picnixz picnixz approved these changes
+1 more reviewer
rruuaanng rruuaanng left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL