-
Notifications
You must be signed in to change notification settings - Fork 42
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
Update cmake files #114
Update cmake files #114
Conversation
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.
Thanks for this. I find working on the build system can be a bit soul-destroying.
@@ -131,21 +131,29 @@ set(CMAKE_Fortran_FLAGS ) | |||
|
|||
add_custom_target(wrapper ALL DEPENDS ${FSOURCES}) | |||
add_custom_command( | |||
OUTPUT SLYCOTmodule.c _wrappermodule.c _wrapper-f2pywrappers.f | |||
OUTPUT _wrappermodule.c _wrapper-f2pywrappers.f |
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.
What happened to SLYCOTmodule.c
?
SLYCOTmodule.c _wrappermodule.c _wrapper-f2pywrappers.f | ||
"${PYTHON_SITE}/numpy/f2py/src/fortranobject.c" | ||
_wrappermodule.c | ||
${PYTHON_SITE}/numpy/f2py/src/fortranobject.c |
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.
Just to check: CMake and scikit-build don't find this file for us, or arrange for an "f2py" target that takes care of this?
${FSOURCES}) | ||
|
||
target_link_libraries(${SLYCOT_MODULE} | ||
${LAPACK_LIBRARIES}) | ||
|
||
target_include_directories( | ||
${SLYCOT_MODULE} PUBLIC | ||
${PYTHON_SITE}/numpy/core/include |
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.
Can we use using ${NumPy_INCLUDE_DIR}
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'm also not sure if these should be PUBLIC
dependencies of the module.
Small fixes to the CMakeLists.txt files.
Now prints out the numpy header path.