-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
CLN: use float64_t consistently instead of double, double_t #23583
Changes from 17 commits
fa38001
af27870
83ca237
367969e
16d75a3
4f4d1cf
d1511f7
c9ef170
1445830
d46d516
f2f4b8d
e7adaf2
03ae9cf
9f51831
33fc12d
8bc71b8
5e50897
52ede45
23344b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,7 @@ from numpy cimport (ndarray, | |
NPY_FLOAT32, NPY_FLOAT64, | ||
NPY_OBJECT, | ||
int8_t, int16_t, int32_t, int64_t, uint8_t, uint16_t, | ||
uint32_t, uint64_t, float32_t, float64_t, | ||
double_t) | ||
uint32_t, uint64_t, float32_t, float64_t) | ||
cnp.import_array() | ||
|
||
|
||
|
@@ -32,10 +31,9 @@ import missing | |
|
||
cdef float64_t FP_ERR = 1e-13 | ||
|
||
cdef double NaN = <double>np.NaN | ||
cdef double nan = NaN | ||
cdef float64_t NaN = <float64_t>np.NaN | ||
|
||
cdef int64_t iNaT = get_nat() | ||
cdef int64_t NPY_NAT = get_nat() | ||
|
||
tiebreakers = { | ||
'average': TIEBREAK_AVERAGE, | ||
|
@@ -197,9 +195,10 @@ def groupsort_indexer(ndarray[int64_t] index, Py_ssize_t ngroups): | |
return result, counts | ||
|
||
|
||
# TODO: redundant with groupby.kth_smallest_c | ||
@cython.boundscheck(False) | ||
@cython.wraparound(False) | ||
cpdef numeric kth_smallest(numeric[:] a, Py_ssize_t k) nogil: | ||
def kth_smallest(numeric[:] a, Py_ssize_t k) -> numeric: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think will have a big perf slowdown There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its never used in cython and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pls test i once changed this (and tried to remove) and was all negative There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indistinguishable: master:
PR:
Similar for other dtypes |
||
cdef: | ||
Py_ssize_t i, j, l, m, n = a.shape[0] | ||
numeric x | ||
|
@@ -812,23 +811,23 @@ def is_monotonic(ndarray[algos_t, ndim=1] arr, bint timelike): | |
n = len(arr) | ||
|
||
if n == 1: | ||
if arr[0] != arr[0] or (timelike and <int64_t>arr[0] == iNaT): | ||
if arr[0] != arr[0] or (timelike and <int64_t>arr[0] == NPY_NAT): | ||
# single value is NaN | ||
return False, False, True | ||
else: | ||
return True, True, True | ||
elif n < 2: | ||
return True, True, True | ||
|
||
if timelike and <int64_t>arr[0] == iNaT: | ||
if timelike and <int64_t>arr[0] == NPY_NAT: | ||
return False, False, True | ||
|
||
if algos_t is not object: | ||
with nogil: | ||
prev = arr[0] | ||
for i in range(1, n): | ||
cur = arr[i] | ||
if timelike and <int64_t>cur == iNaT: | ||
if timelike and <int64_t>cur == NPY_NAT: | ||
is_monotonic_inc = 0 | ||
is_monotonic_dec = 0 | ||
break | ||
|
@@ -853,7 +852,7 @@ def is_monotonic(ndarray[algos_t, ndim=1] arr, bint timelike): | |
prev = arr[0] | ||
for i in range(1, n): | ||
cur = arr[i] | ||
if timelike and <int64_t>cur == iNaT: | ||
if timelike and <int64_t>cur == NPY_NAT: | ||
is_monotonic_inc = 0 | ||
is_monotonic_dec = 0 | ||
break | ||
|
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.
its not actually
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.
OK. The typing is more specific in the groupby version, but the code itself is nearly identical
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.
try to remove and you will see why it’s here
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.
I'll get rid of the comment, but am kind of surprised: you're usually Holy Crusader against redundant code, and the body of this function is very nearly copy/pasted
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.
my point is i don’t think it’s easy to remove
you are more than welcome to try
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.
No you're right. I removed the comment.