Use CMake as primary build system via scikit-build #215
Conversation
Codecov Report
@@ Coverage Diff @@
## main #215 +/- ##
==========================================
- Coverage 93.75% 93.72% -0.03%
==========================================
Files 136 136
Lines 10790 10794 +4
==========================================
+ Hits 10116 10117 +1
- Misses 674 677 +3
Continue to review full report at Codecov.
|
| from numpy.distutils.core import setup | ||
|
|
||
| metadata['configuration'] = configuration | ||
| for i in (Path(__file__).resolve().parent / "_skbuild").rglob("CMakeCache.txt"): |
There was a problem hiding this comment.
This stuff is worth a comment for future generations :)
What is being replaced here and why.
There was a problem hiding this comment.
I'll put a comment there. For reference it is from scikit-build/scikit-build#521; if you build the library locally there is an _skbuild subdirectory in which the cmake cache sits and if you subsequently try to build the library via conda you might get a cache mismatch. But I guess it should be removed as soon as the issue is fixed and a new skbuild release is out.
There was a problem hiding this comment.
So it is already an optimization workaround?
There was a problem hiding this comment.
it's a workaround you only need if you first build the pip package and subsequently a conda package without removing the local build cache first 🙂
There was a problem hiding this comment.
I mean in principle I think a local cache directory should have absolutely no influence on a subsequent conda build but unfortunately it does 😕
There was a problem hiding this comment.
It makes total sense to replace the rudimentary OpenMP and releated detections with the advanced and sophisticated mechanism of CMake/Ninja.
As the project already depend on CMake this transition makes the whole build process more bullet proof and elegant.
I just wonder what the final invocation in setup.py main function really does. It is very obscure to me.
|
I suggest to use pytest-xdist with two jobs on Azure to speed up the testing process. |
|
Thanks for the thorough review! |
No description provided.