GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-125957: sync argument naming in sphinx docs of the cmath and help() by skirpichev · Pull Request #125958 · python/cpython

Conversation

@skirpichev

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Discrepancy in argument naming for Sphinx docs & help in the cmath module #125957

📚 Documentation preview 📚: https://cpython-previews--125958.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
skirpichev added 2 commits October 25, 2024 09:12
@skirpichev
pythongh-125957: sync argument naming in sphinx docs of the cmath and…
769c65f
… help()
@skirpichev
+ 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 bedevere-app Bot mentioned this pull request Oct 25, 2024
Discrepancy in argument naming for Sphinx docs & help in the cmath module #125957
Closed
@bedevere-app bedevere-app Bot added the awaiting review label Oct 25, 2024
@skirpichev skirpichev added the skip news label Oct 25, 2024
@skirpichev

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rruuaanng
rruuaanng reviewed Oct 25, 2024
View reviewed changes
Comment thread Modules/clinic/cmathmodule.c.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

@rruuaanng rruuaanng mentioned this pull request Oct 25, 2024
Poor readability of preprocessor generated by Tools/clinic #125963
Closed
skirpichev added 2 commits February 17, 2025 09:46
@skirpichev
Merge branch 'master' into fix-cmath-sig-125957
680e7bf
@skirpichev
Revert second commit
57cc455
picnixz
picnixz reviewed Feb 17, 2025
View reviewed changes
Comment thread Doc/library/cmath.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/cmath.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev @picnixz
Apply suggestions from code review
ec3754e
Co-authored-by: Bénédikt Tran <[email protected]>
picnixz
picnixz approved these changes Feb 17, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Feb 17, 2025
@picnixz

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)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner approved these changes Feb 17, 2025
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@skirpichev

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)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev skirpichev added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Mar 30, 2025
This was referenced Mar 30, 2025
gh-131885: simplify function signatures in the cmath module #131886
Closed
Add a single table as summary to cmath documentation #131887
Merged
@AA-Turner
AA-Turner merged commit 0a3eb88 into python:main Apr 1, 2025
@miss-islington-app

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.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 1, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 1, 2025
@skirpichev @picnixz @miss-islington
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 <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Apr 1, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 1, 2025
@skirpichev @picnixz @miss-islington
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 <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app 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
@miss-islington @skirpichev @picnixz
[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 <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
AA-Turner pushed a commit that referenced this pull request Apr 1, 2025
@miss-islington @skirpichev @picnixz
[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 <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@skirpichev
skirpichev deleted the fix-cmath-sig-125957 branch April 1, 2025 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@skirpichev @picnixz @vstinner @rruuaanng @AA-Turner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.