GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-131885: simplify function signatures in the cmath module by skirpichev · Pull Request #131886 · python/cpython Conversation
|
| Benchmark | ref | patch |
|---|---|---|
| sin(1.1) | 417 ns | 428 ns: 1.03x slower |
| sin(1j) | 414 ns | 422 ns: 1.02x slower |
| log(2.3) | 390 ns | 409 ns: 1.05x slower |
| log(1+2j) | 449 ns | 468 ns: 1.04x slower |
| log(2.1, 3.2) | 601 ns | 630 ns: 1.05x slower |
| Geometric mean | (ref) | 1.04x slower |
For reviewers:
- this is on top of gh-125957: sync argument naming in sphinx docs of the cmath and help() #125958
- alternative: adjust sphinx docs instead, like gh-101118: correct function signatures in the math module docs #101120
benchmark code
# bench.py import pyperf from cmath import sin, log runner = pyperf.Runner() runner.bench_func('sin(1.1)', sin, 1.1) runner.bench_func('sin(1j)', sin, 1j) runner.bench_func('log(2.3)', log, 2.3) runner.bench_func('log(1+2j)', log, 1+2j) runner.bench_func('log(2.1, 3.2)', log, 2.1, 3.2)
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
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
Co-authored-by: Bénédikt Tran <[email protected]>
Now function docstrings are in sync with the module docs, for example:
```pycon
>>> import cmath, inspect
>>> help(cmath.sin)
Help on built-in function sin in module cmath:
sin(z)
Return the sine of z.
>>> inspect.signature(cmath.sin)
<Signature (z)>
```
As a side effect, this allows `sin(z=1.23)` calls. The price is slight
performance penalty (maybe this can be fixed on AC side):
| Benchmark | ref | patch |
|----------------|:------:|:--------------------:|
| sin(1.1) | 417 ns | 428 ns: 1.03x slower |
| sin(1j) | 414 ns | 422 ns: 1.02x slower |
| log(2.3) | 390 ns | 409 ns: 1.05x slower |
| log(1+2j) | 449 ns | 468 ns: 1.04x slower |
| log(2.1, 3.2) | 601 ns | 630 ns: 1.05x slower |
| Geometric mean | (ref) | 1.04x slower |
AA-Turner
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.
What's the justification for these runtime changes? I don't see any value to being able to use z as a keyword argument, and we shouldn't make things slower just for aesthetics (removing the / in help()).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
It's explained in the pr description and (in depth) issue thread. With this change we have correct signatures in docs, without cluttering them with syntax, which might be hard for newbies.
No, it's not for aesthetics, but for readability. In pure-Python world you have to add extra syntax to have effect of positional-only arguments. This is something people have to learn and it's not common for being in recipes, tutorials, etc. IMO, trade speed for readability is good. Though, maybe not for all cases (e.g. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The signatures in the documentation are already correct, no? The debate over slash and star is over how precisely we describe runtime semantics. Is there something else I'm missing? The documentation should be driven by the runtime, not vice versa. A |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
No. As for many modules, the cmath sphinx docs are out of sync with module docstrings. Alternative to this pr will be adding trailing slashes to sphinx docs.
I think that documentation should be 1) readable 2) accurately describe runtime behavior. To achieve 1) and 2) we can adjust either docs or runtime, or both. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
To be clear though, the only difference is the slash? I wouldn't describe that as 'wrong', but as 'slightly less precise'. On the assumption that the above is true, I'll close this PR. Perhaps there is a JavaScript solution we can use to 'simplify' the documentation for beginners. A |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
@AA-Turner, we have EB decision: "If a function accepts positional-only or keyword-only arguments, include the slash and the star in the signature as appropriate: [...] Although the syntax is terse, it is precise about the allowable ways to call the function and is taken from Python itself." Sorry, I don't see here option for compromises.
Currently stars and slashes are highlighted and tooltips appear in the sphinx docs. Though, same syntax appears e.g. in the help() output without any hints for user... |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
BTW, can you change your mind if speed issue will be solved? |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
2 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.