FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Make pybind11 and nanobind required for benchmarks; fix CI backend detection by Copilot · Pull Request #6 · peter-urban/xtensor-python · GitHub

  1. peter-urban
  2. xtensor-python

Repository navigation

Make pybind11 and nanobind required for benchmarks; fix CI backend detection - #6

Merged
peter-urban merged 5 commits into
xtensor-nanobindfrom
copilot/require-pybind11-nanobind
Feb 8, 2026
ConversationCommits (5)ChecksFiles changed
Merged

Make pybind11 and nanobind required for benchmarks; fix CI backend detection#6
peter-urban merged 5 commits into
xtensor-nanobindfrom
copilot/require-pybind11-nanobind

Conversation

Copilot AI commented Feb 6, 2026 •
edited
Loading

Copy link
Copy Markdown
  • Fix benchmark/run_benchmarks.py: try importing pre-built extensions before attempting setup.py build
  • Fix benchmark/run_benchmarks.py: exit with non-zero when no backends available (sys.exit(0) → sys.exit(1))
  • Fix benchmark/run_benchmarks.py: add --require-all flag to require both pybind11 and nanobind backends
  • Fix .github/workflows/linux.yml: enable nanobind benchmark build, build benchmark targets, run from build dir with --require-all
  • Fix .github/workflows/osx.yml: same changes as linux CI
  • Fix .github/workflows/windows.yml: same changes as linux/osx CI (was missing benchmark build targets and --require-all)
  • Fix macOS build: link xtensor-python on APPLE in benchmark CMakeLists.txt
  • Fix macOS build: narrowing conversion error in benchmark/main_nanobind.cpp:392
  • Fix Windows build: add PRIVATE keyword to MSVC target_link_libraries in benchmark CMakeLists.txt
  • Fix macOS build: narrowing conversion error in benchmark/main.cpp:429
Original prompt

https://github.com/peter-urban/xtensor-python/

Configuring with: cmake . -DCMAKE_BUILD_TYPE=Release -G Ninja
Building with: cmake --build . --config Release
Available backends:
pybind11: [FAIL]
nanobind: [FAIL]
Warning: No backend available! Skipping benchmarks.

obviously i want pybind11 and nanobind to be required
make sure that the benchmark step fails if pybind11 and nanobind can't be found
fix that thhey cant be found
same for macos ci

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Make pybind11 and nanobind required for benchmarks; fix CI detection
7661a62 
- run_benchmarks.py: try importing pre-built extensions before setup.py build
- run_benchmarks.py: exit(1) when no backends available (was exit(0))
- run_benchmarks.py: add --require-all flag for requiring both backends
- linux.yml/osx.yml: enable BUILD_NANOBIND_BENCHMARK, build benchmark targets,
  run from build/benchmark with --require-all

Co-authored-by: peter-urban <3885740+peter-urban@users.noreply.github.com>
Copilot AI changed the title [WIP] Require pybind11 and nanobind for benchmarks Make pybind11 and nanobind required for benchmarks; fix CI backend detection Feb 6, 2026
Copilot AI requested a review from peter-urban February 6, 2026 20:11

peter-urban commented Feb 6, 2026

Copy link
Copy Markdown
Owner

@copilot some tests are failing, please fix

Fix macOS benchmark build: link xtensor-python on APPLE, fix narrowin…
0fee0a7 
…g error

- benchmark/CMakeLists.txt: always link xtensor-python for pybind11
  benchmark target (was missing on APPLE, causing pybind11/pybind11.h
  not found)
- benchmark/main_nanobind.cpp: use from_shape() instead of brace-init
  to fix size_t-to-double narrowing error on macOS

Co-authored-by: peter-urban <3885740+peter-urban@users.noreply.github.com>

Copilot AI commented Feb 6, 2026

Copy link
Copy Markdown
Author

@copilot some tests are failing, please fix

Fixed in 0fee0a7. The macOS builds were failing because:

  1. benchmark/CMakeLists.txt didn't link xtensor-python on APPLE, so the pybind11 benchmark couldn't find pybind11/pybind11.h
  2. benchmark/main_nanobind.cpp:392 had a size_t→double narrowing error — replaced brace-init with from_shape() (matching the pattern used elsewhere in the same file)

peter-urban commented Feb 8, 2026

Copy link
Copy Markdown
Owner

@copilot tests are still failing, please fix

Fix Windows CMake error and macOS narrowing error in benchmark
f9bd778 
- benchmark/CMakeLists.txt: add PRIVATE keyword to MSVC target_link_libraries
  to fix CMake error about mixing keyword/plain signatures
- benchmark/main.cpp:429: use from_shape() instead of brace-init to fix
  size_t-to-double narrowing error on macOS

Co-authored-by: peter-urban <3885740+peter-urban@users.noreply.github.com>

Copilot AI commented Feb 8, 2026

Copy link
Copy Markdown
Author

@copilot tests are still failing, please fix

Fixed in f9bd778. Two issues:

  1. Windows: benchmark/CMakeLists.txt:84 — target_link_libraries for MSVC was missing the PRIVATE keyword, causing a CMake error about mixing keyword/plain signatures (line 80 uses PRIVATE).
  2. macOS 14/15: benchmark/main.cpp:429 — same size_t→double narrowing error as the nanobind file. Replaced brace-init with from_shape().

peter-urban commented Feb 8, 2026

Copy link
Copy Markdown
Owner

@copilot nearly there, pybind11 is still not avaliable for windows benchmarks.

Fix Windows CI: enable benchmark builds with pybind11 and nanobind
c967a1d 
Apply same changes as linux.yml/osx.yml:
- Add -DBUILD_NANOBIND_BENCHMARK=ON to CMake configure
- Build benchmark_xtensor_python and benchmark_xtensor_nanobind targets
- Run benchmarks from build/benchmark with --require-all
- Remove continue-on-error: true

Co-authored-by: peter-urban <3885740+peter-urban@users.noreply.github.com>

Copilot AI commented Feb 8, 2026

Copy link
Copy Markdown
Author

@copilot nearly there, pybind11 is still not avaliable for windows benchmarks.

Fixed in c967a1d. The Windows workflow (windows.yml) was never updated like Linux and OSX were — it was missing -DBUILD_NANOBIND_BENCHMARK=ON, the benchmark build targets, --require-all, and was still running from the source benchmark/ dir with continue-on-error: true. Now all three CI files are consistent.

peter-urban marked this pull request as ready for review February 8, 2026 21:47
peter-urban merged commit 830604c into xtensor-nanobind Feb 8, 2026
22 of 24 checks passed
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

peter-urban Awaiting requested review from peter-urban

Assignees

peter-urban

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL