-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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
Use CMake for Build System #47380
Use CMake for Build System #47380
Conversation
I tried this on Windows. Doing Right now, cmake is not working, but I'm not sure I have the latest build tools installed. |
Yea it could be. I didn't put a lot of time into that yet but if you want to explore would be great. I ideally want CMake to build the Cython extensions. That way running This is hard coded right now for Linux. For windows / your python version, you likely need to change like 8 of the top level CMake file to match the expected file ending for your platform:
This might be abstracted in some form via the CMake Python variables. If not something we can branch on in our file and upstream: https://cmake.org/cmake/help/latest/module/FindPython.html#result-variables |
Well, I'm not sure I have the time nor understanding to get this working on Windows! |
Yah, i expect this would be easier to get working on Mac, but it still involves a learning curve. I'm fine embracing that for personal edification, curious how others feel |
Ah sorry...didn't think about make not working on Windows. I think you can do cmake .
cmake --build . On all systems to get this to work for an "in source" build |
So now that I'm on Visual Studio 2019 build tools, there is an issue with the cmake version when doing
The version of Secondly, |
I think we can drop the minimum CMake version to 3.14 which would help. I just pulled the 3.22 off their documentation, but the NumPy finder was added in 3.14 For cleaning, you could ideally do |
I made the following changes to
Then I discovered that you have to specify the architecture to cmake. So I did If I don't include Don't know ANYTHING about cmake to figure out how to get the files out of those Release subdirectories into the right place. Also not sure that the |
Awesome! I think you can tweak those settings via some CMAKE variables: |
FYI the idiomatic approach to CMake is to do an "out of source" build, where we would have a dedicated build sub folder and all required elements would be copied into that. In such a case, I don't think the Release / Debug sub folders generated by VS would matter much. However, we today do in source builds of pandas. So want to make sure we emulate that as best as possible for initial lift and shift |
Once you figure that out for Linux, I hope it will be straightforward to do for Windows. I'll wait to hear about your progress. |
Sounds good. Thanks for the feedback so far - very helpful |
I'd be supportive of trying CMake. Generally hoping
|
Not the latter - scikit-learn uses setuptools, and is most likely to move to Meson (work is planned to start in the coming month). scikit-image and others scikits I can think off all still use either setuptools or numpy.distutils |
Ah thanks for clarifying. I thought the scikit-build package was associated with scikit-learn |
What's the criterion for judging this? Is it just LOC? I think there's an implicit "avoiding setuptools is desirable" consensus that I'm out of the loop on. |
Setuptools invokes the compiler in an ad-hoc fashion, it's difficult to parallelize and very difficult to cross-compile. Those are among the bigger reasons SciPy switched from distutils to Meson. The same argument would generally apply to switching from distutils/setuptools to cmake, disregarding personal preferences between Meson and cmake. |
To help clarify some more I think this is also worth drafting up via the new PDEP process @datapythonista is working on. This PR is part research, discussion and implementation at this point, so probably a good fit for that new process |
Yeah, I think it is worth pursing a PDEP. There's a lot of insight in the comments of this PR that I don't want to get lost, and are helpful when someone wants to revisit the build system or figure out rationale behind certain decisions. GitHub tends to handle PRs with lots of comments/activity super horribly, and kind of randomly collapses comments that could be important in the middle. I think it is also worth pursuing a parallel PR using meson as the build backend. I am willing to do that if no one beats me to it. EDIT: Will start tomorrow. |
@jbrockmendel for a bit more context:
For new projects, the only situation where you'd still want to prefer |
Hello everyone, I haven't really commented much the progress on using Meson as pandas' build system, but I now can compile basically all the c extensions. I'm still working on enabling some more extensions and cleaning up a lot of the boilerplate code, but I will try opening a draft PR next week. The next step would be to build wheels/figure out how to install locally. I'm still working through some gnarly issues in meson-python for that. All in all, we should probably start thinking of drafting up the PDEP sometime soon. |
can you confirm we would still be migrating to cibuildwheel #44027 to build wheels? #44027 (comment) |
Both meson and this Cmake implementation use PEP 517 build hooks, so cibuildwheel would build the wheels the same way(through build). |
My WIP meson work can be found here lithomas1#19. |
@Dr-Irv any insight on the typing failure in CI? Does this stem from pandas-stubs having an entry for pandas._libs.json? Not sure how those should be synced Otherwise I think this PR is in a usable state. Started on PDEP but lost a little motivation, and this was pretty close so decided to finish this instead |
I'm hazarding a guess here, but I think that you didn't include any of the |
Thanks @Dr-Irv makes sense. Was overthinking that one This is all green now if its something we'd still entertain (the Docker failure can't pass until this is merged into main). There's a lot more we can do to improve our build system (moving to out of source builds comes to mind first and foremost) but I think this represents a reasonable lift and shift |
Hey @WillAyd, I'm happy to help you draft up the PDEP if you need it. |
OK sure just created #47988 for that |
After discussion seems like Meson is the way to go. Let's close this one |
Proof of concept. I think we can greatly simplify our building system using this. @mroeschke @lithomas1 @Dr-Irv
This is in an intermediate state, but you can do:
make clean
will clean up build artifacts, which right now excludes the generated Cython files but should include them in the future.Have the following TODOs:
[ ]: The CMAKE_SHARED_LIBRARY_SUFFIX is hard-coded to assume Py38 on linux
[ ]: Get rid of
python setup.py
and have CMake perform cythonization[ ]: Update CI, documentation, etc...