ci: modernize build, add macOS support, and enable python tests#78
Merged
Conversation
actions/upload-artifact@v3 and actions/download-artifact@v3 stopped working on 2025-01-30, which is why the PyPI workflow has been failing in 7 seconds on every push. Upgrade all action versions to current releases and adapt pypi.yml to the v4 unique-name requirement. Also drop the dead INCLUDE_PYTHON_WRAPPER cmake.arg (the variable is unused and produces a warning) and pin cmake.version to <4 to insulate the wheel build from CMake 4.x deprecations in transitive deps. - actions/checkout: v3 -> v4 - actions/setup-python: v4 -> v5 - actions/upload-artifact: v3 -> v4 (with explicit names) - actions/download-artifact: v3 -> v4 (pattern + merge-multiple) - pre-commit/action: v3.0.0 -> v3.0.1 - pypa/cibuildwheel: v2.16.5 -> v2.21.3 - drop jwlawson/actions-setup-cmake (runners now ship cmake >= 3.27)
The cpp/python builds work cleanly on macOS in local testing — the algorithm core only depends on Eigen (FetchContent-bundled) and uses no OpenMP/TBB, so AppleClang is sufficient and no brew toolchain is required. Add macos-14 (Apple Silicon) and macos-13 (Intel) to: - cpp.yml::cpp_api (cpp_api_dev stays Linux-only by design — apt deps) - python.yml::python_package - pypi.yml::cibuildwheel (so release tags publish mac wheels too) fail-fast: false so a flake on one OS doesn't cancel the rest.
The previous Makefile assumed GNU coreutils (nproc --all), which fails on macOS. Detect the host OS and route the parallel build count through sysctl on Darwin, nproc elsewhere. Also drop the redundant 'pip install numpy' from the pyinstall target — numpy is already declared in pyproject.toml as a runtime dependency, so pip resolves it automatically. README: add the macOS prerequisite block (brew install cmake) and extend the Tested Environment list to cover macOS 13/14 and Ubuntu 24.04.
cpp/cmake/pybind11.cmake was never include()'d anywhere — python/ uses find_package(pybind11 CONFIG REQUIRED) directly, and cpp/ does not need pybind11 at all. The pinned v2.2.3 (2018) was just noise. Activate the previously-commented pytest step in python.yml and add two minimal smoke tests under python/tests/: one verifies the imported module exposes the public API, the other runs a real ground estimation against data/000000.bin and asserts the output partitions the input. Wire up a [test] extra in pyproject.toml so the CI step can install pytest with the package in one command.
GitHub-hosted macos-13 (Intel) runners have severely limited capacity — jobs sat queued for 1h 43m on the test PR without ever starting. Apple is winding down Intel Mac and runner availability is unlikely to improve. Restrict the macOS coverage to macos-14 (Apple Silicon) so the matrix is reliably green; Intel mac users can still build locally.
a2a3890 to
c119846
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Modernizes the build/CI surface so the repo is green again and works on macOS. Bundled as one PR per the maintainer's request, but each commit is independently reviewable and can be cherry-picked.
The PyPI workflow has been failing in 7 seconds on every push since 2025-01-30 because GitHub deprecated
actions/upload-artifact@v3. This PR fixes that, and also closes out the long-standing# TODO(hlim): Support Maccomments in the workflows.Commits
ci: upgrade deprecated GitHub Actions and clean pyproject— bumpscheckout/setup-python/upload-artifact/download-artifact/cibuildwheel/pre-committo current versions, adaptspypi.ymlto v4's unique-name requirement, drops deadINCLUDE_PYTHON_WRAPPERcmake.arg, pinscmake.version<4for the wheel build.ci: extend matrix with macOS runners (arm64 + Intel)— addsmacos-14andmacos-13tocpp_api,python_package, andcibuildwheeljobs. The algorithm core only depends on Eigen (FetchContent-bundled) and uses no OpenMP/TBB, so AppleClang is sufficient and no brew toolchain is required.build: make Makefile cross-platform (macOS + Linux)—nproc --alldoesn't exist on macOS; route the parallel-build count throughsysctlon Darwin. Drop the redundantpip install numpy(pyproject already declares it). Add a macOS prerequisite block to the README.chore: drop dead pybind11.cmake and add a python smoke test—cpp/cmake/pybind11.cmakewas neverinclude()'d; the v2.2.3 (2018) pin was just noise. Adds two minimal pytest smoke tests (test_smoke.py) covering API exposure and a real ground-estimation run againstdata/000000.bin, and activates the previously-commented pytest step inpython.yml.Test plan
make cppinstallsucceeds on macOS arm64 (AppleClang 17, CMake 3.31)pip install ./pythonproduces a workingpypatchworkppwheel on macOS arm64 (Python 3.12)pip install './python[test]' && pytest python/— 2 tests pass