Build Python extensions with CMake - #3350
Conversation
|
✔️ 83ba919 -> Azure artifacts URL |
|
✔️ 6028ba5 -> Azure artifacts URL |
|
✔️ f118ff2 -> Azure artifacts URL |
|
✔️ 26f31f0 -> Azure artifacts URL |
Also add MPT headers when running in CI
|
✔️ 30b45e7 -> Azure artifacts URL |
There was a problem hiding this comment.
Thanks! This is a great benefit and represents substantial modernization. The main comment clarifies very well. I'm looking forward to working with this new style. Any issues that crop up in the future with my sometimes peculiar build circumstances are best fixed with future PRs on this base. I tried
hines@Michaels-MacBook-Pro-2 temp % bash bldnrnmacpkg.sh python3.9 python3.13 2>&1 | tee ztemp
and of course it fails with a dozen or so
/Users/hines/neuron/temp/build/share/lib/python/neuron/rxd/geometry3d/graphicsPrimitives.cpp:30453:134: error: use of undeclared identifier '_PyGC_FINALIZED'
/Users/hines/neuron/temp/build/share/lib/python/neuron/rxd/geometry3d/graphicsPrimitives.cpp:33500:36: error: no member named 'curexc_traceback' in '_ts'
/Users/hines/neuron/temp/build/share/lib/python/neuron/rxd/geometry3d/graphicsPrimitives.cpp:34259:55: error: no member named 'ob_digit' in '_longobject'
/Users/hines/neuron/temp/build/share/lib/python/neuron/rxd/geometry3d/graphicsPrimitives.cpp:34400:27: error: no matching function for call to '_PyLong_AsByteArray'
Go ahead and merge. I'll deal with this next week.
|
|
✔️ 537f40f -> Azure artifacts URL |
|
@nrnhines I tried running |



Context
We currently build NEURON Python modules with distutils (integrated as part of setuptools), which has been marked deprecated for years (see also #2228). It is not at all obvious how any of the Python modules are built, and tracing how it actually works (how the CLI args are constructed) basically requires stepping through
setup.pyusing PDB (the Python debugger). distutils also has very poor compiler support (see for instance #3313), so it would be useful to move the Python compiled modules to another build system. Ideally, we would migrate everything to something likescikit-build-core, which basically just runs CMake and builds the Python module.Changes
This PR moves the following modules so they get built with CMake instead, and use
scikit-build-coreas a build backend:neuron.hocneuron.rxd.geometry3dneuronmusic(not shipped in wheels)This has the following consequences:
setup.pyand any other files that it used (likeMANIFEST.inorsetup.cfg) are removedpyproject.tomlfilepip wheel --no-deps .cibuildwheeland running it in the top-level directorybuild_wheels.bashscript can now be used in almost exactly the same way as before, except thecoreneuronoption has been replaced by theNRN_ENABLE_CORENEURONenv variable (or passing--config-settings=cmake.define.NRN_ENABLE_CORENEURON=ONtopip, see updated dev docs for details)pyproject.toml, notably, the name of the wheel (this has nothing to do with the importable Python package name, just the filename of the wheel and the metadata that is inserted). Thechange_name.pyscript is added solely as a workaround for this limitation (otherwise we would need to add yet anotherCMakeLists.txtsomewhere that just configures thepyproject.tomlfile, and CMake-in-CMake is not a pleasant experience), and runs only when doing a release build (admittedly, it is hard to test that the change in Azure actually works, though I can verify that at least the script itself works locally)*.pyfiles) is a good ideaPythonCompileHelper.cmake, are thecythonizeandadd_nrn_python_libraryfunctions, which are extensively documented, and now build the above modulesfind_package(Python)now has to find NumPy headers as well. This is not a problem when building wheels, but whenNRN_PYTHON_DYNAMIChas multiple versions, each Python version must be able to find its own installation of NumPyNRN_INSTALL_PYTHON_PREFIX, and anything else (scripts, libraries, etc.) is installed underNRN_INSTALL_DATA_PREFIX. The two variables are internal and cannot be set, but default tolib/python/neuronand.(for CMake builds), andneuronandneuron/.data/(for wheels). This is because we currently ship all of the other non-Python NEURON components underneuron/.dataEDIT: requires merging neuronsimulator/iv#55 first.