bpo-37691: Let math.dist() accept sequences and iterables for coordinates #14975
Conversation
rhettinger
added
type-feature
| double diffs_on_stack[NUM_STACK_ELEMS]; | ||
| double *diffs = diffs_on_stack; | ||
|
|
||
| if (!PyTuple_Check(p)) { |
There was a problem hiding this comment.
Why not call just PySequence_Tuple() (which is a no-op for tuples)? This would simplify the setup and the cleanup code.
It may be also worth to use PySequence_Fast() and PySequence_Fast_GET_ITEM() in the loop. This will save you allocating a new tuple. For general iterable PySequence_Tuple() creates a list and a tuple, but PySequence_Fast() creates just a list.
But you will need to call PySequence_Fast_GET_SIZE() in a loop because list's size can be changed when you convert items to the C double.
There was a problem hiding this comment.
I didn't want any external function calls on the fast path for the common case.
I did try it out. The patch was shorter but it caused a 15% performance degradation:
# baseline
$ pytime -r11 -s 'from math import dist' 'dist((10.5, 12.5), (13.5, 8.5))'
5000000 loops, best of 11: 57.3 nsec per loop
# patched to always call PySequence_Tuple() and always Py_DECREF on exit
$ pytime -r11 -s 'from math import dist' -s 'p, q = (10.5, 12.5), (13.5, 8.5)' 'dist(p, q)'
5000000 loops, best of 11: 64.7 nsec per loop
|
Thanks @rhettinger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8. |
|
GH-14984 is a backport of this pull request to the 3.8 branch. |
…ates (pythonGH-14975) (cherry picked from commit 6b5f1b4) Co-authored-by: Raymond Hettinger <[email protected]>
…ates (GH-14975) (GH-14984) (cherry picked from commit 6b5f1b4) Co-authored-by: Raymond Hettinger <[email protected]>
5 participants
https://bugs.python.org/issue37691