Skip to content

Add Python venv setup script and fix pyproject.toml #322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
krystophny wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Python venv setup script and fix pyproject.toml #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,4 @@ thirdparty/pyplot_module.F90
.claude
docs/_build/
build*/
.venv/
8 changes: 7 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ NVHPC_HPCX := $(NVHPC_ROOT)/comm_libs/13.0/hpcx/hpcx-2.25.1/ompi
NVHPC_BUILD_DIR := build_nvfortran
NVHPC_ACC_BUILD_DIR := build_nvfortran_acc

.PHONY: all configure reconfigure build build-deterministic build-deterministic-nopy test test-nopy test-fast test-slow test-regression test-all test-golden-main test-golden-tag test-golden install clean nvfortran nvfortran-test nvfortran-test-nopy nvfortran-configure nvfortran-clean
.PHONY: all configure reconfigure build build-deterministic build-deterministic-nopy test test-nopy test-fast test-slow test-regression test-all test-golden-main test-golden-tag test-golden install clean venv nvfortran nvfortran-test nvfortran-test-nopy nvfortran-configure nvfortran-clean
.PHONY: nvfortran-acc nvfortran-acc-test nvfortran-acc-test-nopy nvfortran-acc-configure nvfortran-acc-clean
all: build

Expand Down Expand Up @@ -98,6 +98,12 @@ test-golden: build-deterministic-nopy
test-golden-fast: build
cd $(BUILD_DIR) && ctest --output-on-failure $(if $(filter 1,$(VERBOSE)),-V) -L "golden_record" $(if $(TEST),-R "$(TEST)")

venv:
./setup-venv.sh

venv-nopy:
./setup-venv.sh --no-pysimple

doc: configure
cmake --build --preset default --target doc

Expand Down
3 changes: 0 additions & 3 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,3 @@ cmake.define.CMAKE_CACHE_DIR = "build"
wheel.packages = ["python/pysimple"]
wheel.install-dir = "pysimple"
wheel.exclude = []

[project.package-data]
pysimple = ["data/*.dat"]
25 changes: 13 additions & 12 deletions requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
# Python dependencies for SIMPLE
# Pin exact versions for reproducibility in CI
# Use minimum versions for compatibility across Python releases.
# Exact pins break on newer Python versions that lack prebuilt wheels.

# Core dependencies
numpy==2.2.6
scikit-build-core==0.10.0
numpy>=2.2
scikit-build-core>=0.10

# F90wrap - pinned to release version
# F90wrap - pinned to release version (API-sensitive)
f90wrap==0.2.16

# Optional dependencies for testing
pytest==7.4.3
pytest-cov==6.2.1
netCDF4==1.7.2
# Testing
pytest>=7.4
pytest-cov>=6.0
netCDF4>=1.7

# Optional dependencies for examples/plotting
matplotlib==3.9.4
scipy==1.15.2
shapely==2.0.5
# Plotting and examples
matplotlib>=3.9
scipy>=1.15
shapely>=2.0
65 changes: 65 additions & 0 deletions setup-venv.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#!/bin/bash
# Create and populate a Python virtual environment for SIMPLE.
# Usage: ./setup-venv.sh [--no-pysimple]
#
# Options:
# --no-pysimple Skip building pysimple (Fortran-Python bindings).
# Use this if you only need Python test/plot dependencies.
#
# Prerequisites (system packages):
# - python3 (>= 3.9) with venv module
# - gfortran, cmake, ninja-build
# - libnetcdff-dev (or netcdf-fortran-devel), liblapack-dev
# See docs/PREREQUISITES.md for distro-specific install commands.

set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
VENV_DIR="${SCRIPT_DIR}/.venv"
BUILD_PYSIMPLE=1

for arg in "$@"; do
case "$arg" in
--no-pysimple) BUILD_PYSIMPLE=0 ;;
-h|--help)
sed -n '2,/^$/s/^# //p' "$0"
exit 0
;;
*)
echo "Unknown option: $arg" >&2
exit 1
;;
esac
done

echo "Creating virtual environment in ${VENV_DIR} ..."
python3 -m venv "$VENV_DIR"
# shellcheck disable=SC1091
source "${VENV_DIR}/bin/activate"

echo "Upgrading pip ..."
pip install --upgrade pip

echo "Installing Python dependencies from requirements.txt ..."
pip install --prefer-binary -r "${SCRIPT_DIR}/requirements.txt"

if [ "$BUILD_PYSIMPLE" -eq 1 ]; then
echo "Building pysimple (Fortran-Python bindings) ..."
# Build SIMPLE first if not already built
if [ ! -f "${SCRIPT_DIR}/build/build.ninja" ]; then
echo " Configuring CMake ..."
cmake -S "$SCRIPT_DIR" -B"${SCRIPT_DIR}/build" -GNinja \
-DCMAKE_BUILD_TYPE=Release -DCMAKE_COLOR_DIAGNOSTICS=ON
fi
echo " Building Fortran library ..."
cmake --build "${SCRIPT_DIR}/build" --config Release

echo " Installing pysimple in editable mode ..."
pip install --no-build-isolation -e "$SCRIPT_DIR"
Comment on lines +49 to +58
Copy link
Copy Markdown

@qodo-code-review qodo-code-review bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Stale cmake disables bindings 🐞 Bug ✓ Correctness

setup-venv.sh only configures CMake when build/build.ninja is missing, so an existing build/
configured with ENABLE_PYTHON_INTERFACE=OFF can be reused and pysimple won’t be built correctly.
This can lead to pip install -e . producing a package that fails to import because the python/
subdirectory is skipped by CMake.
Agent Prompt 
### Issue description
`setup-venv.sh` reuses an existing `build/` directory if `build/build.ninja` exists, without ensuring `-DENABLE_PYTHON_INTERFACE=ON`. If a developer previously ran `make build-deterministic-nopy` (which configures `build/` with `ENABLE_PYTHON_INTERFACE=OFF`), the venv setup will reuse that cache and the Python bindings won’t be built/installed correctly.

### Issue Context
- `Makefile` has a `build-deterministic-nopy` target that configures `build/` with `-DENABLE_PYTHON_INTERFACE=OFF`.
- Top-level `CMakeLists.txt` only adds `python/` when `ENABLE_PYTHON_INTERFACE` is enabled.
- `pyproject.toml` sets scikit-build-core `build-dir = "build"`, so pip editable installs will interact with the same build directory.

### Fix Focus Areas
- setup-venv.sh[46-58]
- Makefile[73-85]
- pyproject.toml[13-19]

### Implementation direction
Do one of the following (preferred options first):
1. Use a dedicated build directory for the venv workflow (e.g. `build_venv/`), so it cannot conflict with `make build-deterministic-nopy`.
2. Always run an explicit reconfigure step when `BUILD_PYSIMPLE=1`, passing `-DENABLE_PYTHON_INTERFACE=ON` (and optionally `-DSIMPLE_ENABLE_PYTHON_TOOLS=ON`) even when `build/build.ninja` exists.
3. Detect a cached `CMakeCache.txt` with `ENABLE_PYTHON_INTERFACE:BOOL=OFF` and either error out with a clear message or automatically reconfigure/clean.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

else
echo "Skipping pysimple build (--no-pysimple)."
fi

echo ""
echo "Done. Activate with:"
echo " source .venv/bin/activate"
Loading