-
-
Notifications
You must be signed in to change notification settings - Fork 26k
MAINT Use C99 functions from Cython #15098
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
sklearn/neighbors/quad_tree.pyx
Outdated
# XXX using (size_t)(-1) is ugly, but SIZE_MAX is not available in C89 | ||
# (i.e., older MSVC). | ||
cdef SIZE_t DEFAULT = <SIZE_t>(-1) | ||
cdef SIZE_t DEFAULT = SIZE_MAX |
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 think the referenced SIZE_MAX
is the one from stdint.h
but I would appreciate is someone could double-check.
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 can confirm it is defined there: https://en.cppreference.com/w/c/types/limits
Can we use SIZE_MAX
directly in this file instead of defining a DEFAULT
?
sklearn/neighbors/quad_tree.pyx
Outdated
@@ -279,7 +275,7 @@ cdef class _QuadTree: | |||
cdef bint res = True | |||
for i in range(self.n_dimensions): | |||
# Use EPSILON to avoid numerical error that would overgrow the tree | |||
res &= fabsf(point1[i] - point2[i]) <= EPSILON | |||
res &= fabs(point1[i] - point2[i]) <= EPSILON |
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.
we're using double not instead of float? is there no fabsf in libc.math
?
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 mistake. fabsf
is in math.h
but for some reason not re-exported in cython's libc.math
I think. Reverted that particular change.
Sure done. Another easy and related PR is #15097 |
Thank you @rth ! |
Removes some hacks and re-implementations for older MSVC compilers than should no longer be necessary since we dropped Python <3.5