Transition to pyproject.toml

[JCGoran]

Overview and context of changes

This PR moves all of the build logic from the dynamic setup.py into the new static pyproject.toml configuration file, as described in PEP 621. Since NMODL has a handy Python wrapper, it makes sense to use a modern Python build system.

Some of the notable changes and explanations for them:

• wheels can now be built using pip wheel . --no-deps, with an optional --wheel-dir [DIR] for output
• the actual building is done by scikit-build-core, and any options can be passed by adding -C [OPTION]=[VALUE] to the above call (on MacOS, this is needed to specify the build dir as it fails in a temp dir due to a bug in cmake)
• remove setup.py and all various requirements files in favor of pyproject.toml (except a single requirements.txt which has all of them too)
• move some setup.cfg options to pyproject.toml as well (notably flake8 and sphinx do not support it, so we leave those out for now)
• due to the fact that pip doesn't provide a way of parsing the pyproject.toml file using the CLI, to install all dependencies (build, run, optional), we need to do pip install 'pip-tools>=7.4.0', followed by pip install -r <(pip-compile --all-build-deps --all-extras --no-strip-extras 2>&1) Note that this will also leave a requirements.txt in your current dir (not sure if this is a feature or a bug of pip-compile) use pip install -r requirements.txt to install requirements
• move documentation generation from setup.py to a standalone script packaging/generate_docs.sh which generates the documentation (see docs for usage)
• pip install -e . (i.e. editable install) doesn't quite work due to the strange way we bundle everything, but pip install -C build-dir=[SOMEDIR] is fast enough so there isn't much of a difference. If we later merge Move Python bindings to own dir #1179 we can also sidestep any issues with imports
• add NMODL_BUILD_WHEEL to cmake config so we can signal we are building a wheel (this sets the appropriate flags automatically)
• remove random include and lib stuff from wheel because they're not needed (looking at you, fmtlib and eigen). Also fixes fmtlib places files outside of .data when not linking against Python #1153
• due to the limitations of PEP 621, it's not allowed to change the name of the package dynamically (we build nmodl-nightly wheels sometimes), so as a workaround there is the packaging/change_name.py script, which can be used for changing the name of the package (only relevant in the CI, so users shouldn't touch it)
• pyproject.toml doesn't allow an empty version (we now call cmake after Python reads pyproject.toml), so we can set it dynamically using setuptools_scm (uses git tags), or even statically using SETUPTOOLS_SCM_PRETEND_VERSION (we seem to use something like git describe --tags | cut -d '-' -f 1,2 | tr - . to set it) in the CI (I didn't want to do too much refactoring with versions)
• pywheel dir is gone, and the _binwrapper script is moved to nmodl dir with minimal changes so we can actually set the right script target
• Python bindings are now in python/nmodl instead of nmodl. Fixes Move nmodl python files into subdirectory #462.
• NMODL_WHEEL_VERSION was removed; if you want to specify a custom version of a wheel, use SETUPTOOLS_SCM_PRETEND_VERSION instead
• finally, update docs with all of the above changes

Stuff that works

• pip wheel . (and python -m build --wheel) actually builds an installable wheel. Note that the latter needs the build package to work properly (but this should only be ran in the CI anyway, most developers should use pip install --config-setting="build-dir=_build" -e . or similar for development and testing).

Stuff that needs work

• figure out how to use a single source of truth for the version, and where to put it; setuptools-scm seems to be able to do it without any file (see here), which is nice, but is Python-specific. Maybe this project? (EDIT: as a workaround, we can set the env variable SETUPTOOLS_SCM_PRETEND_VERSION_FOR_NMODL="$(git describe --tags | cut -d '-' -f 1,2 | tr - .)" to mimic the current behavior)
• the keys build-system.requires and project.dependencies have some duplication: figure out which ones are required when (build-time vs. run-time) (EDIT: separated the two into disjoint sets now, works as intended)
• possibly related to the above: for some reason, most of the other build-system.requires dependencies are also built in wheelhouse/ when running pip wheel (EDIT: it seems pip wheel can be replaced by python -m build --wheel, which doesn't have this issue EDIT 2: the --no-deps flag does exactly that so we don't need build at all)
• the documentation used to be built by setup.py, figure out how to build it without burdening setup.py with it. sphinx comes with the sphinx-build CLI utility, but is a bit cumbersome to use (EDIT: the docs are a bit difficult to refactor since they mix the version and NMODL installation. EDIT 2: preliminary docs seem to work)
• a superset of the documentation issue: where do we put all of the "other" (testing, etc.) dependencies? (EDIT: seems this should go somewhere like project.optional-dependencies.test and project.optional-dependencies.docs)
• bundle various mod files and other package data when creating the wheel
• figure out how to change the name of the package dynamically so we can have both NMODL and NMODL-nightly on PyPI. Note that PEP 621, which defines the pyproject.toml spec, explicitly mentions: "A build back-end MUST raise an error if the metadata specifies the name in dynamic.", so we can't use something that respects that PEP. A possible solution is to have a script in the CI which edits pyproject.toml to change the name of the project to nmodl-nightly (EDIT: as a PoC workaround, added a change_name.py script to the packaging/ subdirectory, that only runs in the CI, which can change the name of the project before we actually build it).
• figure out whether editable installs are supported (which would allow incremental builds) (EDIT: seems that pip install -e . works, and setuptools has support for PEP 660) (EDIT 2: scikit-build does not support editable installs, but scikit-build-core does. Note that one should explicitly set --config-setting="build-dir=[PATH]" to re-use the build directory since build frontends delete them).
  • TODO: maybe pin the earliest version of setuptools which has support for PEP 660? we don't need this if using scikit-build-core
• for some reason, if build-system.build-dir is not specified on MacOS, the build consistently fails (reproducible both in the CI and locally), with the complaint from bison that one of the files cannot be found. Not sure if this is a huge problem (we can always override this by specifying the SKBUILD_BUILD_DIR env variable), but it's a bit annoying that it doesn't work OOTB (EDIT: this is possibly due to this bug in cmake itself; as a workaround, if specifying a temporary dir as a build dir, one can use realpath $(mktemp -d) instead of just mktemp -d)
• the cmake file apparently puts all of the fmt library-related stuff in the install_manifest.txt, which is impossible to ignore using either sdist.exclude or wheel.exclude, so, right now, scikit-build-core puts spurious include and lib (or lib64 on Linux) directories in the wheel which cannot be removed (ticket: fmtlib places files outside of .data when not linking against Python #1153) (EDIT: we can disable this using set(FMT_INSTALL OFF) in CMakeLists.txt. EDIT 2: c.f. Fix standard default installation target presence fmtlib/fmt#3264 maybe there's a more clever way of doing this?)
• when it comes to editable installs, it would be nice if there was a way to install the build-system.requires dependencies using pip (since it vendors its own version of a TOML parser), but I am not very keen on being dependent on dubious enhancements to upstream. Of course, for Python 3.11+, it's possible to make a script which parses pyproject.toml natively that can then feed a list to either stdout or to a requirements.txt file (EDIT: this is now possible with a PoC script in packaging/show_deps.py, which uses the vendored tomli package in pip itself, or the native tomllib if running on Python 3.11. Note that there's also pip-tools, but I haven't found a way for it to output the build-system.requires dependencies. EDIT 2: once this commit gets into a release, then we should be able to use --all-build-deps to get those as well. EDIT 3: pip-tools 7.4.0 and above can now use pip-compile --all-build-deps to get the build-time dependencies. This can also be used with the --all-extras to get all dependencies installed)
  • to clarify the above, to install all of the dependencies using pip-tools, one must run pip install 'pip-tools>=7.4.0' followed by pip install -r <(pip-compile --all-build-deps --all-extras --no-strip-extras 2>&1)
• figure out whether pybind can be removed as an external dependency that we keep track of here (see https://pybind11.readthedocs.io/en/stable/compiling.html#pep-518-requirements-pip-10-required)
• maybe fix Spack recipe as the gitlab pipeline is now failing (EDIT: after py-findlibpython, nmodl: new package because nmodl depends on it spack#2320 gets merged we should be good to go)

[1uc]

I get the following version information for this branch:

$ pip install .
$ python -c "import nmodl; print(nmodl.__version__)"
0.6

is that intended?

[JCGoran]

I get the following version information for this branch:

$ pip install .
$ python -c "import nmodl; print(nmodl.__version__)"
0.6

is that intended?

We set __version__ in Python from whatever CMake says the version is, I haven't touched that; if this is not intended, I think it should go in a separate issue/PR.

[bbpbuildbot]

Logfiles from GitLab pipeline #198340 (:white_check_mark:) have been uploaded here!

Status and direct links:

• ✅ spack_setup
• ✅ build:intel
• ✅ build:nvhpc
• ✅ test:intel
• ✅ test:nvhpc

[1uc]

Let's summarize:

• It installs on two desktops/laptops (Mac and Linux) and CI.
• Docs build on the same two machines and CI.
• There's the surprise that tests use a partial installation of NMODL; and we don't know if it must be like that because in reality NMODL actual is two different Python modules; or if NMODL is one, regular Python module but there's a bug/shortcut in the tests leading to the strange behaviour.
• Certain things were postponed, have the corresponding issues been created?
• Does it build on BB5?
• IIUC the spack recipe has been updated, does it still work?

[JCGoran]

To answer your questions:

Certain things were postponed, have the corresponding issues been created?

For the version issue, see #1199.
Regarding the test failing if we explicitly load the Python module, I don't think it's an issue with NMODL, but rather I with the compiler flags: we try to use the address sanitizer library, but cannot load it dynamically for some reason:

ImportError while importing test module '/home/runner/work/nmodl/nmodl/test/unit/ode/test_ode.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
../../../test/unit/ode/test_ode.py:6: in <module>
    from nmodl.ode import differentiate2c, integrate2c
../../lib/nmodl/__init__.py:34: in <module>
    from ._nmodl import NmodlDriver, to_json, to_nmodl, __version__  # noqa
E   ImportError: libclang_rt.ubsan_standalone-x86_64.so: cannot open shared object file: No such file or directory

We don't ship asan in the distributed wheels, so I don't think it's a big issue. Also, the failing test (ODE) is actually tested in the CI as part of the Pytest suite.

EDIT: issue is here now for posterity #1200

Does it build on BB5?

Yes.

IIUC the spack recipe has been updated, does it still work?

Ongoing discussion at BlueBrain/spack#2332, but I have no strong opinion whether we should still support Python 3.8 or not.
Note that this PR does not introduce any new build or runtime dependencies because the Spack recipe uses CMake as the build system, which I haven't modified.

[1uc]

whether we should still support Python 3.8 or not.

BlueBrain/spack (as opposed to spack/spack) is by and large a bespoke version of Spack for BB5 (and maybe some EPFL controlled laptops). Therefore, there's a reasonably chance that avoiding the issue now means never encountering it. When someone does need Python 3.8, they'll first be sent to the archive of modulse and if that doesn't work one can put in the time to support 3.8.

W.r.t the PR itself: nice work! Since the Spack PR does nothing for [email protected]: we can merge.