MAINT: Ensure _argmaxima1d doesn't keep unneeded memory.#8499
MAINT: Ensure _argmaxima1d doesn't keep unneeded memory.#8499larsoner merged 2 commits intoscipy:masterfrom
Conversation
| # Preallocate, there can't be more maxima than half the size of `x` | ||
| cdef np.ndarray[np.int64_t, ndim=1] maxima | ||
| maxima = np.empty(x.shape[0] // 2, dtype=np.int64) | ||
| maxima = np.empty(x.shape[0] // 2, dtype=np.intp) |
There was a problem hiding this comment.
Why prefer intp (platform-specific?) over int64 here?
There was a problem hiding this comment.
It's the numpy equivalent of Py_ssize_t, and is the standard type of any index returning function in numpy: will not waste memory on 32 bit systems, and will work without overflow risk when 128 bit systems take over!
There was a problem hiding this comment.
In that case, shouldn't the declaration in the line above have the same dtype np.intp_t? Maybe the build failures are related to that?
ValueError: Buffer dtype mismatch, expected 'int64_t' but got 'int'
There was a problem hiding this comment.
Yes indeed, have submitted a new commit fixing that.
lagru
left a comment
There was a problem hiding this comment.
Yes, looks like a clear improvement to me.
|
I pushed a PEP8 fix to make Travis happy, will merge once I see the CIs come back green unless someone else beats me to it. |
e970419 to
a817187
Compare
|
That last commit seems to make all the CIs happy. |
|
Thanks @jaimefrio |
|
My pleasure! |
* MAINT: Ensure _argmaxima1d doesn't keep unneeded memory. * Fix maxima index array type mismatch.
Call
.resizeon the return array, rather than taking a view, to prevent memory being unnecessarily held by the array.