-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
PERF: Using Numpy C-API arange #32681
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
Conversation
Since the perf numbers are pretty similar, can you report a few rounds of each |
Sure!
|
thats a nice improvement for the small cases, thanks |
pandas/_libs/internals.pyx
Outdated
@@ -105,7 +107,9 @@ cdef class BlockPlacement: | |||
Py_ssize_t start, stop, end, _ | |||
if not self._has_array: | |||
start, stop, step, _ = slice_get_indices_ex(self._as_slice) | |||
self._as_array = np.arange(start, stop, step, dtype=np.int64) | |||
# NOTE: this is the C-optimized equivalent of | |||
# np.arange(start, stop, step, dtype=np.int64) |
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.
nitpick: i like to add an extra space at the beginning of multi-line comments specifically to avoid confusing this for commented-out code. quotation marks or backticks or ... are also good
lgtm, thanks for accomodating my nitpicking |
@jbrockmendel I have a list of your nitpicks and requests, I try to address them when I have the time :) |
Why does this generate anything different from Cython? Shouldn't it generate the same result? |
The generated C code has to call np.arange in python-space (and lookup np.int64, and ...) |
So doesn’t work like the built in range call then huh? Was expecting it would automatically convert to C without the Python lookup
…Sent from my iPhone
On Mar 13, 2020, at 3:34 PM, jbrockmendel ***@***.***> wrote:
Why does this generate anything different from Cython? Shouldn't it generate the same result?
The generated C code has to call np.arange in python-space (and lookup np.int64, and ...)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
thanks I agree we should not just wholesale convert calls to use the c-api, but in specific cases where we are actually returning a numpy array (as opposed to iteration), ok |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
This PR was opened as @jbrockmendel suggested (ref #32177 (comment))
Benchmarks:
Master:
PR: