New Build System#35
Conversation
New: CMakeLists.txt: Setup to build GTE. This file is calleed by the python build system (scikit-build). environment.yml: Dependencies for automatic conda environment creation. extern/CoverageKinematics: Updated submodule reference for automated kcl build. Modified: pyproject.toml: Added missing dependencies. Updated to use scikit-build for building gte.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new build/packaging flow to support building and installing the project (including its C++ components) via pip install, with a companion conda environment file to provision native dependencies.
Changes:
- Switched Python build backend to
scikit-build-coreand updated project/development dependencies inpyproject.toml. - Added
environment.ymlto create a conda environment with native dependencies. - Added a top-level
CMakeLists.txtand updatedREADME.mdinstallation instructions for the new workflow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| pyproject.toml | Moves to scikit-build-core and updates packaging metadata/dependencies. |
| environment.yml | Defines the conda environment intended to support local builds/installs. |
| README.md | Documents the new conda + pip installation flow. |
| CMakeLists.txt | Adds CMake entry point to build/install the C++ targets used by Python. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - python=3.13 | ||
| - eigen | ||
| - fmt | ||
| - tbb |
There was a problem hiding this comment.
The environment file sets up runtime deps but does not include common build tooling needed for a CMake/pybind11 build (e.g., cmake, ninja, and a C++ compiler toolchain packages such as cxx-compiler on conda-forge). With the current README instructions (conda env create then pip install .), builds can fail on clean machines. Consider adding the build tools here to make the one-command setup work reliably.
| - tbb | |
| - tbb | |
| - cmake | |
| - ninja | |
| - cxx-compiler |
| Create a conda environment from the provided environment file. | ||
|
|
||
| ``` | ||
| conda env create -f environment.yml | ||
| conda activate orbitpy | ||
| ``` | ||
|
|
||
| Then, install the package into the environment using pip. | ||
|
|
||
| Create a conda environment and install the dependencies: | ||
| ``` | ||
| conda create -n eosim-revised python=3.13 | ||
| conda activate eosim-revised | ||
| conda install sphinx | ||
| pip install sphinx-rtd-theme | ||
| pip install pylint | ||
| pip install black | ||
| pip install coverage | ||
| pip install skyfield | ||
| pip install astropy | ||
| pip install . | ||
| ``` |
There was a problem hiding this comment.
The install instructions don’t mention initializing the extern/CoverageKinematics git submodule. Since the build calls add_subdirectory(extern/CoverageKinematics), pip install . from a fresh clone without submodules will fail. Add a note to clone with --recurse-submodules or run git submodule update --init --recursive before the pip install step.
| install(TARGETS GeometricTools LIBRARY DESTINATION .) | ||
| install(TARGETS kcl LIBRARY DESTINATION .) No newline at end of file |
There was a problem hiding this comment.
The install(TARGETS ...) commands only specify LIBRARY DESTINATION. On Windows, Python extension modules (.pyd) are installed via the RUNTIME destination, so they may not be included in the install/wheel. Add RUNTIME DESTINATION (and consider ARCHIVE DESTINATION if needed) to ensure the targets are installed on all platforms.
| install(TARGETS GeometricTools LIBRARY DESTINATION .) | |
| install(TARGETS kcl LIBRARY DESTINATION .) | |
| install(TARGETS GeometricTools | |
| LIBRARY DESTINATION . | |
| RUNTIME DESTINATION . | |
| ARCHIVE DESTINATION . | |
| ) | |
| install(TARGETS kcl | |
| LIBRARY DESTINATION . | |
| RUNTIME DESTINATION . | |
| ARCHIVE DESTINATION . | |
| ) |
| ] | ||
|
|
||
| [tool.scikit-build] | ||
| wheel.packages = ["src/orbitpy"] |
There was a problem hiding this comment.
[tool.scikit-build] wheel.packages points to src/orbitpy, but this repository uses a top-level orbitpy/ package directory (there is no src/ directory). This will cause the wheel to omit the orbitpy package. Update the packages setting to match the actual package location (e.g., orbitpy).
| wheel.packages = ["src/orbitpy"] | |
| wheel.packages = ["orbitpy"] |
| include = ["orbitpy"] | ||
| requires = [ | ||
| "scikit-build-core>=0.10", | ||
| "pybind11>=2.12" |
There was a problem hiding this comment.
The build backend is CMake-based (scikit-build-core) and the top-level CMakeLists.txt requires CMake >= 3.20, but cmake (and typically ninja) are not listed in [build-system].requires. On a machine without system CMake/Ninja, pip install . will fail while building the wheel. Add explicit build requirements (or otherwise ensure they are provided) so pip builds are reproducible.
| "pybind11>=2.12" | |
| "pybind11>=2.12", | |
| "cmake>=3.20", | |
| "ninja" |
| version = "0.1.0" | ||
| description = "A Python package for computing satellite remote-sensing orbit data." | ||
| readme = "README.md" | ||
| license = "MIT" |
There was a problem hiding this comment.
license = "MIT" is not PEP 621-compliant for the [project] table (the license field is a table, typically license = { text = "MIT" } or license = { file = "LICENSE" }). Some build backends/metadata parsers will reject this and fail the build.
| license = "MIT" | |
| license = { text = "MIT" } |
Added new build system so that entire library, included kcl, can be installed with a single pip install. Dependencies are managed using a conda environment which is specified in an environment.yml for easy setup.
New:
Modified: