-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Added np.get_include() to Cython autowrapper, fixes #8847. #8848
Conversation
If you look at
The one in the past didn't, but it also didn't work. How would we support arrays without it? |
"\n" | ||
"setup(\n" | ||
" cmdclass = {{'build_ext': build_ext}},\n" | ||
" ext_modules = [Extension({ext_args}, extra_compile_args=['-std=c99'])]\n" | ||
" ext_modules = [Extension({ext_args}, extra_compile_args=['-std=c99'])],\n" | ||
" include_dirs = [np.get_include()],\n" |
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.
Maybe the numpy
lines of the template should also depend on _need_numpy
? Assuming that that bit of code works the way I intended, this would allow scalar expressions to be compiled without NumPy being installed.
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, I didn't notice _need_numpy
. I'll make it dependent on that.
- Made CodeWrapper a new style class. - Moved _need_numpy into __init__ so it doesn't pollute other instances. - Optionally include numpy include directories in Cython setup.py.
fad508c
to
1615390
Compare
I verified this works on Mac OSX. @jcrist once tests pass and you approve I'll merge. |
Looks good to me. |
Added np.get_include() to Cython autowrapper, fixes #8847.
@jcrist This fixes this issue for MacOSX (and probably anyone that has the NumPy headers in an odd place). But after doing this, I was wondering why we have numpy calls in the CythonWrapper. Seems like you should be able to Cythonize sympy expressions without having numpy installed. Did the CythonWrapper always require numpy? Maybe we really need two Cython wrappers, one with the numpy headers and another without.