♻️ Replace pybind11 with nanobind#514
Conversation
denialhaag
added
python
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughMigrates Python/C++ bindings from pybind11 to nanobind across CI, build configuration, CMake, and binding sources; updates mqt.core pins to 3.4.0 and related hashes; removes SKBUILD-specific pybind11 handling; makes AnnotatableQuantumComputation non-copyable. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@bindings/bindings.cpp`:
- Around line 133-136: The defaulted move constructor pythonbuf(pythonbuf&&) is
unsafe because std::streambuf copies internal pointers while d_buffer ownership
transfers, causing the moved-from object’s destructor (~pythonbuf) to call
_sync() on freed memory; fix by deleting the move constructor (declare
pythonbuf(pythonbuf&&) = delete;) so pythonbuf cannot be moved, or if moves are
required implement a custom move that transfers d_buffer and nulls the source
d_buffer and resets the source streambuf pointers before leaving the source in a
safe-to-destroy state; update the declaration near ~pythonbuf and _sync()
references accordingly.
In `@cmake/ExternalDependencies.cmake`:
- Around line 28-32: The nanobind discovery should mirror the mqt-core pattern:
update the execute_process(...) call that sets nanobind_ROOT (and capture
RESULT_VARIABLE) and then check that nanobind_ROOT is non-empty before invoking
find_package; if it is empty emit a clear message and optionally call
message(FATAL_ERROR ...) or skip with message(STATUS ...); also add ERROR_QUIET
to the find_package(nanobind CONFIG REQUIRED) invocation (or use
find_package(nanobind CONFIG REQUIRED QUIET/ERROR_QUIET depending on desired
behavior) so CMake doesn't print confusing output when nanobind is missing.
Ensure you reference the variables nanobind_ROOT, Python_EXECUTABLE,
execute_process, and find_package in the change.
In `@include/core/annotatable_quantum_computation.hpp`:
- Around line 483-485: Move the deleted copy constructor and assignment operator
for AnnotatableQuantumComputation from the protected section into the public
section (place them right after the explicit constructor declaration) so the
declarations AnnotatableQuantumComputation(const AnnotatableQuantumComputation&)
= delete; and AnnotatableQuantumComputation& operator=(const
AnnotatableQuantumComputation&) = delete; are publicly visible, improving
compiler diagnostics while preserving non-copyable semantics.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/ci.ymlCMakeLists.txtbindings/CMakeLists.txtbindings/bindings.cppcmake/ExternalDependencies.cmakeinclude/core/annotatable_quantum_computation.hpppyproject.toml
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qcec PR: 817
File: pyproject.toml:81-82
Timestamp: 2026-01-09T17:58:10.350Z
Learning: In the Munich Quantum Toolkit projects using nanobind, setting `wheel.py-api = "cp312"` in `[tool.scikit-build]` enables Stable ABI wheels only for Python 3.12+ (where nanobind supports it), while automatically building regular non-ABI3 wheels for earlier Python versions (3.10, 3.11) and free-threading builds (3.14t). This allows a single configuration to appropriately handle both old and new Python versions without forcing incompatible ABI requirements.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/dd/register_matrix_dds.cpp:64-109
Timestamp: 2025-12-15T01:54:22.129Z
Learning: In the munich-quantum-toolkit/core repository, after migrating to nanobind, docstrings for Python bindings are now added directly in the C++ binding code (using R"pb(...)pb" syntax) and stub files (.pyi) are auto-generated using the `bindings/generate-stubs.sh` script. This replaces the previous pybind11 approach where docstrings were manually maintained in stub files.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: bindings/fomac/fomac.cpp:227-264
Timestamp: 2025-12-07T01:21:27.544Z
Learning: In the munich-quantum-toolkit/core repository, docstrings for Python bindings are added to the corresponding stub files (.pyi) rather than directly in the pybind11 C++ bindings code. This practice may change if the project adopts nanobind with automatic stub generation.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/ir/register_permutation.cpp:153-171
Timestamp: 2025-12-22T01:25:21.609Z
Learning: In the munich-quantum-toolkit/core repository, when using nanobind iterator factory functions like `make_key_iterator` and `make_iterator`, the unqualified form (without explicit `nb::` prefix) is preferred. The clang-tidy configuration suggests removal of explicit namespace qualification, relying on ADL (Argument-Dependent Lookup) to resolve these functions correctly.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:111-116
Timestamp: 2025-12-19T00:05:54.428Z
Learning: In the munich-quantum-toolkit/core repository after migrating to nanobind, lifetime management differs from pybind11: `nb::keep_alive<nurse, patient>()` does not exist in nanobind. Instead, use `nb::rv_policy::reference_internal` when binding methods that return objects referencing internal state of the parent object (e.g., Session::getDevices returning Device objects that depend on the Session). This tells nanobind to keep the parent alive while children exist.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:348-364
Timestamp: 2025-12-21T22:35:08.572Z
Learning: In the munich-quantum-toolkit/core repository's nanobind bindings, use `.sig("...")` on parameter arguments that have vector or container defaults (e.g., `"sites"_a.sig("...") = std::vector<fomac::Session::Device::Site>{}`) to prevent exposing mutable defaults in the Python API, which would be flagged as a code smell by Python linters. This pattern is preferred over removing `.sig("...")` even though it shows `...` in the stub signature.
📚 Learning: 2025-12-15T01:54:22.129Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/dd/register_matrix_dds.cpp:64-109
Timestamp: 2025-12-15T01:54:22.129Z
Learning: In the munich-quantum-toolkit/core repository, after migrating to nanobind, docstrings for Python bindings are now added directly in the C++ binding code (using R"pb(...)pb" syntax) and stub files (.pyi) are auto-generated using the `bindings/generate-stubs.sh` script. This replaces the previous pybind11 approach where docstrings were manually maintained in stub files.
Applied to files:
bindings/CMakeLists.txt.github/workflows/ci.ymlpyproject.tomlcmake/ExternalDependencies.cmakebindings/bindings.cpp
📚 Learning: 2025-12-07T01:21:27.544Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: bindings/fomac/fomac.cpp:227-264
Timestamp: 2025-12-07T01:21:27.544Z
Learning: In the munich-quantum-toolkit/core repository, docstrings for Python bindings are added to the corresponding stub files (.pyi) rather than directly in the pybind11 C++ bindings code. This practice may change if the project adopts nanobind with automatic stub generation.
Applied to files:
bindings/CMakeLists.txt.github/workflows/ci.ymlbindings/bindings.cpp
📚 Learning: 2026-01-09T17:58:10.350Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qcec PR: 817
File: pyproject.toml:81-82
Timestamp: 2026-01-09T17:58:10.350Z
Learning: In the Munich Quantum Toolkit projects using nanobind, setting `wheel.py-api = "cp312"` in `[tool.scikit-build]` enables Stable ABI wheels only for Python 3.12+ (where nanobind supports it), while automatically building regular non-ABI3 wheels for earlier Python versions (3.10, 3.11) and free-threading builds (3.14t). This allows a single configuration to appropriately handle both old and new Python versions without forcing incompatible ABI requirements.
Applied to files:
bindings/CMakeLists.txt.github/workflows/ci.ymlpyproject.tomlcmake/ExternalDependencies.cmake
📚 Learning: 2025-12-21T22:35:08.572Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:348-364
Timestamp: 2025-12-21T22:35:08.572Z
Learning: In the munich-quantum-toolkit/core repository's nanobind bindings, use `.sig("...")` on parameter arguments that have vector or container defaults (e.g., `"sites"_a.sig("...") = std::vector<fomac::Session::Device::Site>{}`) to prevent exposing mutable defaults in the Python API, which would be flagged as a code smell by Python linters. This pattern is preferred over removing `.sig("...")` even though it shows `...` in the stub signature.
Applied to files:
bindings/CMakeLists.txtbindings/bindings.cpp
📚 Learning: 2025-12-15T01:59:17.023Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Applied to files:
bindings/CMakeLists.txt.github/workflows/ci.ymlbindings/bindings.cpp
📚 Learning: 2025-12-28T17:14:53.890Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: src/qdmi/na/CMakeLists.txt:31-38
Timestamp: 2025-12-28T17:14:53.890Z
Learning: In the munich-quantum-toolkit/core repository, the NA device generator target (mqt-core-qdmi-na-device-gen) is intentionally propagated to MQT_CORE_TARGETS via list(APPEND) because it's publicly linked to the NA device library (the NA device uses a public function from the generator). The SC device generator is not propagated because it has no such dependency with the SC device library.
Applied to files:
bindings/CMakeLists.txtcmake/ExternalDependencies.cmake
📚 Learning: 2025-10-10T08:09:54.528Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: bindings/CMakeLists.txt:0-0
Timestamp: 2025-10-10T08:09:54.528Z
Learning: In the Munich Quantum Toolkit (MQT) Core project, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which automatically prefixes all CMake `DESTINATION` paths with `mqt/core/` during wheel installation. Therefore, CMake install destinations are relative to the `mqt/core` package namespace, not `site-packages`.
Applied to files:
bindings/CMakeLists.txt.github/workflows/ci.ymlpyproject.tomlcmake/ExternalDependencies.cmake
📚 Learning: 2025-12-14T16:51:52.504Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 23
File: .readthedocs.yaml:13-18
Timestamp: 2025-12-14T16:51:52.504Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, LLVM and MLIR toolchains are required for the documentation build because `uv run` includes a full build of the package, which compiles C++/MLIR extensions using scikit-build-core.
Applied to files:
.github/workflows/ci.ymlpyproject.toml
📚 Learning: 2025-10-09T13:14:10.178Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:219-221
Timestamp: 2025-10-09T13:14:10.178Z
Learning: The MQT Core project (munich-quantum-toolkit/core repository) uses the C++20 standard, not C++17. C++20 features such as abbreviated function templates (e.g., `const auto&` parameters) are supported and valid in this codebase.
Applied to files:
.github/workflows/ci.ymlcmake/ExternalDependencies.cmake
📚 Learning: 2025-12-13T20:08:45.549Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qmap PR: 862
File: pyproject.toml:65-66
Timestamp: 2025-12-13T20:08:45.549Z
Learning: In the qmap project (pyproject.toml), maintain broad compatibility with dependencies across supported Python versions. Avoid artificially raising minimum version requirements unless there's a specific need (e.g., to guarantee binary wheel availability for certain Python versions, or to access required features). The goal is to keep the software as broadly compatible as possible with the rest of the ecosystem.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-28T17:13:36.900Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: pyproject.toml:98-102
Timestamp: 2025-12-28T17:13:36.900Z
Learning: In the munich-quantum-toolkit/core project, scikit-build-core is intelligent enough to skip build targets listed in pyproject.toml that don't exist for a given platform, so platform-specific targets (like `-dyn` targets conditioned on `NOT WIN32`) can be unconditionally listed in `build.targets` without causing Windows build failures.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-10T08:10:16.394Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: test/python/na/test_na_fomac.py:35-0
Timestamp: 2025-10-10T08:10:16.394Z
Learning: In the munich-quantum-toolkit/core repository, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which means CMake `install()` commands with `DESTINATION <path>` install files relative to `mqt/core/` in the wheel, making them accessible via `files("mqt.core").joinpath("<path>")`.
Applied to files:
pyproject.tomlcmake/ExternalDependencies.cmake
📚 Learning: 2025-11-04T15:22:19.558Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1243
File: test/python/qdmi/qiskit/conftest.py:155-157
Timestamp: 2025-11-04T15:22:19.558Z
Learning: The munich-quantum-toolkit/core repository requires Python 3.10 or later, so Python 3.10+ features (such as `zip(..., strict=...)`, pattern matching, etc.) are acceptable and should not be flagged as compatibility issues.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-11T19:39:32.050Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/debugger PR: 160
File: pyproject.toml:54-54
Timestamp: 2025-10-11T19:39:32.050Z
Learning: Qiskit packages use cp39-abi3 wheels (stable ABI) which are forward-compatible with Python 3.9+ including Python 3.14, even if the package classifiers don't explicitly list Python 3.14 support.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-09T22:15:59.924Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1246
File: pyproject.toml:340-341
Timestamp: 2025-10-09T22:15:59.924Z
Learning: Qiskit publishes ABI3 wheels (e.g., cp39-abi3) that are forward-compatible with newer Python versions including Python 3.14, so no explicit Python 3.14 wheels are required for qiskit to work on Python 3.14.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-05T17:45:37.602Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
Applied to files:
cmake/ExternalDependencies.cmake
📚 Learning: 2025-12-01T11:00:40.342Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: CHANGELOG.md:18-18
Timestamp: 2025-12-01T11:00:40.342Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the CHANGELOG.md intentionally references the parent MQT Core repository's release notes (https://github.com/munich-quantum-toolkit/core/releases) because the plugin repository is based on work performed in the parent repository.
Applied to files:
cmake/ExternalDependencies.cmake
📚 Learning: 2025-12-05T15:57:39.701Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 3
File: lib/Conversion/CatalystQuantumToMQTOpt/CMakeLists.txt:22-25
Timestamp: 2025-12-05T15:57:39.701Z
Learning: The munich-quantum-toolkit projects (core and core-plugins-catalyst) use `file(GLOB_RECURSE ...)` patterns in CMakeLists.txt files to collect header files, following an established convention in the parent repository for consistency across the codebase.
Applied to files:
cmake/ExternalDependencies.cmake
📚 Learning: 2025-12-19T00:05:54.428Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:111-116
Timestamp: 2025-12-19T00:05:54.428Z
Learning: In the munich-quantum-toolkit/core repository after migrating to nanobind, lifetime management differs from pybind11: `nb::keep_alive<nurse, patient>()` does not exist in nanobind. Instead, use `nb::rv_policy::reference_internal` when binding methods that return objects referencing internal state of the parent object (e.g., Session::getDevices returning Device objects that depend on the Session). This tells nanobind to keep the parent alive while children exist.
Applied to files:
bindings/bindings.cpp
📚 Learning: 2025-12-22T01:25:21.609Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/ir/register_permutation.cpp:153-171
Timestamp: 2025-12-22T01:25:21.609Z
Learning: In the munich-quantum-toolkit/core repository, when using nanobind iterator factory functions like `make_key_iterator` and `make_iterator`, the unqualified form (without explicit `nb::` prefix) is preferred. The clang-tidy configuration suggests removal of explicit namespace qualification, relying on ADL (Argument-Dependent Lookup) to resolve these functions correctly.
Applied to files:
bindings/bindings.cpp
🪛 Cppcheck (2.19.0)
bindings/bindings.cpp
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingInclude)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[style] 47-47: The function 'getCurrentAggregateStatementExecutionOrderState' is never used.
(unusedFunction)
[style] 141-141: The function 'none' is never used.
(unusedFunction)
[style] 57-57: The function 'addParameter' is never used.
(unusedFunction)
[style] 68-68: The function 'findParameterOrVariable' is never used.
(unusedFunction)
[style] 101-101: The function 'variableName' is never used.
(unusedFunction)
[style] 105-105: The function 'constantExpression' is never used.
(unusedFunction)
[style] 109-109: The function 'evaluate' is never used.
(unusedFunction)
[style] 41-41: The function 'truncateConstantValueToExpectedBitwidth' is never used.
(unusedFunction)
[style] 29-29: The function 'modules' is never used.
(unusedFunction)
[style] 33-33: The function 'findModule' is never used.
(unusedFunction)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🇨 Test 🍎 (macos-14, clang, Debug) / 🍎 macos-14 clang Debug
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (macos-14) / 🐍 macos-14
- GitHub Check: 🇨 Test 🍎 (macos-14, clang, Release) / 🍎 macos-14 clang Release
- GitHub Check: 🇨 Test 🏁 (windows-2022, msvc, Release) / 🏁 windows-2022 msvc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🇨 Coverage / 📈 Coverage
- GitHub Check: 🇨 Lint / 🚨 Lint
🔇 Additional comments (12)
pyproject.toml (4)
3-6: LGTM! Build dependencies correctly updated for nanobind migration.The replacement of pybind11 with nanobind and the mqt.core version bump to 3.4.0 are consistent with the PR objectives.
67-68: Stable ABI configuration correctly enables cp312+ ABI3 wheels.Based on learnings from the Munich Quantum Toolkit projects, setting
wheel.py-api = "cp312"enables Stable ABI wheels only for Python 3.12+ (where nanobind supports it), while automatically building regular non-ABI3 wheels for earlier Python versions (3.10, 3.11) and free-threading builds. This is the correct configuration.
292-295: Shared library exclusions correctly updated for mqt-core 3.4.The SOVERSION in the exclusion list matches the mqt.core 3.4.0 dependency, ensuring the mqt-core shared libraries are not incorrectly vendorized into the wheel.
309-312: Good addition of ABI3 audit for Stable ABI wheels.The
abi3audit --strict --reportstep validates that cp312+ wheels correctly comply with the Stable ABI, catching any violations during the build process. Usinginherit.repair-wheel-command = "append"correctly chains this after the platform-specific wheel repair..github/workflows/ci.yml (1)
94-94: LGTM! CI linting correctly updated to install nanobind.The version 2.10.2 matches the minimum requirement specified in pyproject.toml, ensuring consistent builds across CI and local development.
CMakeLists.txt (1)
42-43: LGTM! Python find_package correctly updated for Stable ABI support.The
${SKBUILD_SABI_COMPONENT}variable is set by scikit-build-core when building Stable ABI wheels (e.g., toDevelopment.SABIModulefor cp312+). Including it directly in the COMPONENTS list is the correct approach—when empty for non-ABI3 builds, it has no effect; when set, it ensures the SABIModule component is found.bindings/CMakeLists.txt (1)
28-38: LGTM! CMake binding function correctly switched to nanobind variant.The change from
add_mqt_python_bindingtoadd_mqt_python_binding_nanobindaligns with the project-wide migration to nanobind. All existing parameters are preserved, minimizing build configuration disruption. The function is provided by mqt-core 3.4.0, which is fetched and made available viaFetchContent_MakeAvailable()in ExternalDependencies.cmake.cmake/ExternalDependencies.cmake (1)
36-40: Version bump to mqt-core 3.4.0 looks correct.The version update aligns with the nanobind migration in mqt-core. Based on the retrieved learnings, mqt-core 3.4.0 includes the nanobind-based bindings that this PR depends on.
bindings/bindings.cpp (4)
26-36: Header migration to nanobind looks correct.The nanobind STL binding headers are correctly included with NOLINT comments for the include-cleaner, as these are used implicitly for type conversions. The namespace setup follows the conventions from the munich-quantum-toolkit learnings.
139-165: Stream redirect implementation follows nanobind patterns correctly.The custom
scoped_ostream_redirectandscoped_estream_redirectappropriately replace pybind11's built-in equivalents. The implementation from the referenced nanobind discussion handles UTF-8 properly and works correctly withcall_guard.Note: The same move-constructor concern applies here—if moved, the stream would point to an invalid buffer. Since
call_guarddoesn't move its guards, this is safe in practice.
169-201: Class bindings correctly migrated to nanobind.The conversion follows nanobind patterns correctly:
nb::rv_policy::reference_internalon line 183 correctly handles the__getitem__returning a reference, ensuring the parent's lifetime. Based on learnings, this is the nanobind replacement for pybind11'skeep_alive.nb::enum_withoutexport_values()is intentional—nanobind enums are scoped by default.
203-243: Remaining bindings and module functions look correct.
nb::overload_castusage on lines 209-210 correctly handles overloadedsetmethods.nb::call_guard<scoped_estream_redirect>()on lines 241-242 properly redirects stderr for synthesis functions.- All
def_rwanddef_prop_rousages follow nanobind conventions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@bindings/bindings.cpp`:
- Around line 154-159: The scoped_ostream_redirect constructor is incorrectly
marked noexcept even though initializing the member buffer (buffer(pyostream))
can throw (e.g., bad_alloc or Python exceptions); remove the noexcept specifier
from the scoped_ostream_redirect constructor declaration/definition so
exceptions can propagate (or alternatively catch and translate initializer
exceptions inside the constructor body), updating any matching declarations for
scoped_ostream_redirect to no longer be noexcept while keeping the
destructor/other methods' exception guarantees unchanged.
- Line 164: The defaulted move constructor
scoped_ostream_redirect(scoped_ostream_redirect&&) should be removed because it
inherits pythonbuf's unsafe move of the buffer and leaves both moved-from and
moved-to instances holding the same old pointer and costream reference, risking
double-restore in the destructor; delete this move constructor from
scoped_ostream_redirect (leave copy/move operations disabled) since call_guard
constructs in-place and moves are unnecessary.
♻️ Duplicate comments (1)
bindings/bindings.cpp (1)
141-141: Move constructor remains unsafe.This issue was previously flagged. The defaulted move constructor is problematic because
std::streambufcopies its internal pointers on move whiled_bufferownership transfers. Consider deleting it as suggested in the prior review.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
bindings/bindings.cpp
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/dd/register_matrix_dds.cpp:64-109
Timestamp: 2025-12-15T01:54:22.129Z
Learning: In the munich-quantum-toolkit/core repository, after migrating to nanobind, docstrings for Python bindings are now added directly in the C++ binding code (using R"pb(...)pb" syntax) and stub files (.pyi) are auto-generated using the `bindings/generate-stubs.sh` script. This replaces the previous pybind11 approach where docstrings were manually maintained in stub files.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/qcec PR: 817
File: pyproject.toml:81-82
Timestamp: 2026-01-09T17:58:10.350Z
Learning: In the Munich Quantum Toolkit projects using nanobind, setting `wheel.py-api = "cp312"` in `[tool.scikit-build]` enables Stable ABI wheels only for Python 3.12+ (where nanobind supports it), while automatically building regular non-ABI3 wheels for earlier Python versions (3.10, 3.11) and free-threading builds (3.14t). This allows a single configuration to appropriately handle both old and new Python versions without forcing incompatible ABI requirements.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/ir/register_permutation.cpp:153-171
Timestamp: 2025-12-22T01:25:21.609Z
Learning: In the munich-quantum-toolkit/core repository, when using nanobind iterator factory functions like `make_key_iterator` and `make_iterator`, the unqualified form (without explicit `nb::` prefix) is preferred. The clang-tidy configuration suggests removal of explicit namespace qualification, relying on ADL (Argument-Dependent Lookup) to resolve these functions correctly.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: bindings/fomac/fomac.cpp:227-264
Timestamp: 2025-12-07T01:21:27.544Z
Learning: In the munich-quantum-toolkit/core repository, docstrings for Python bindings are added to the corresponding stub files (.pyi) rather than directly in the pybind11 C++ bindings code. This practice may change if the project adopts nanobind with automatic stub generation.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:111-116
Timestamp: 2025-12-19T00:05:54.428Z
Learning: In the munich-quantum-toolkit/core repository after migrating to nanobind, lifetime management differs from pybind11: `nb::keep_alive<nurse, patient>()` does not exist in nanobind. Instead, use `nb::rv_policy::reference_internal` when binding methods that return objects referencing internal state of the parent object (e.g., Session::getDevices returning Device objects that depend on the Session). This tells nanobind to keep the parent alive while children exist.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:348-364
Timestamp: 2025-12-21T22:35:08.572Z
Learning: In the munich-quantum-toolkit/core repository's nanobind bindings, use `.sig("...")` on parameter arguments that have vector or container defaults (e.g., `"sites"_a.sig("...") = std::vector<fomac::Session::Device::Site>{}`) to prevent exposing mutable defaults in the Python API, which would be flagged as a code smell by Python linters. This pattern is preferred over removing `.sig("...")` even though it shows `...` in the stub signature.
📚 Learning: 2025-12-15T01:54:22.129Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/dd/register_matrix_dds.cpp:64-109
Timestamp: 2025-12-15T01:54:22.129Z
Learning: In the munich-quantum-toolkit/core repository, after migrating to nanobind, docstrings for Python bindings are now added directly in the C++ binding code (using R"pb(...)pb" syntax) and stub files (.pyi) are auto-generated using the `bindings/generate-stubs.sh` script. This replaces the previous pybind11 approach where docstrings were manually maintained in stub files.
Applied to files:
bindings/bindings.cpp
📚 Learning: 2025-12-07T01:21:27.544Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: bindings/fomac/fomac.cpp:227-264
Timestamp: 2025-12-07T01:21:27.544Z
Learning: In the munich-quantum-toolkit/core repository, docstrings for Python bindings are added to the corresponding stub files (.pyi) rather than directly in the pybind11 C++ bindings code. This practice may change if the project adopts nanobind with automatic stub generation.
Applied to files:
bindings/bindings.cpp
📚 Learning: 2025-12-22T01:25:21.609Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/ir/register_permutation.cpp:153-171
Timestamp: 2025-12-22T01:25:21.609Z
Learning: In the munich-quantum-toolkit/core repository, when using nanobind iterator factory functions like `make_key_iterator` and `make_iterator`, the unqualified form (without explicit `nb::` prefix) is preferred. The clang-tidy configuration suggests removal of explicit namespace qualification, relying on ADL (Argument-Dependent Lookup) to resolve these functions correctly.
Applied to files:
bindings/bindings.cpp
📚 Learning: 2025-12-19T00:05:54.428Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:111-116
Timestamp: 2025-12-19T00:05:54.428Z
Learning: In the munich-quantum-toolkit/core repository after migrating to nanobind, lifetime management differs from pybind11: `nb::keep_alive<nurse, patient>()` does not exist in nanobind. Instead, use `nb::rv_policy::reference_internal` when binding methods that return objects referencing internal state of the parent object (e.g., Session::getDevices returning Device objects that depend on the Session). This tells nanobind to keep the parent alive while children exist.
Applied to files:
bindings/bindings.cpp
📚 Learning: 2025-12-15T01:59:17.023Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1383
File: python/mqt/core/ir/operations.pyi:9-16
Timestamp: 2025-12-15T01:59:17.023Z
Learning: In the munich-quantum-toolkit/core repository, stub files (.pyi) are auto-generated by nanobind's stubgen tool and should not be manually modified for style preferences, as changes would be overwritten during regeneration.
Applied to files:
bindings/bindings.cpp
📚 Learning: 2025-12-21T22:35:08.572Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/fomac/fomac.cpp:348-364
Timestamp: 2025-12-21T22:35:08.572Z
Learning: In the munich-quantum-toolkit/core repository's nanobind bindings, use `.sig("...")` on parameter arguments that have vector or container defaults (e.g., `"sites"_a.sig("...") = std::vector<fomac::Session::Device::Site>{}`) to prevent exposing mutable defaults in the Python API, which would be flagged as a code smell by Python linters. This pattern is preferred over removing `.sig("...")` even though it shows `...` in the stub signature.
Applied to files:
bindings/bindings.cpp
🪛 Cppcheck (2.19.0)
bindings/bindings.cpp
[information] 23-23: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingInclude)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 47-47: The function 'getCurrentAggregateStatementExecutionOrderState' is never used.
(unusedFunction)
[style] 141-141: The function 'none' is never used.
(unusedFunction)
[style] 57-57: The function 'addParameter' is never used.
(unusedFunction)
[style] 68-68: The function 'findParameterOrVariable' is never used.
(unusedFunction)
[style] 101-101: The function 'variableName' is never used.
(unusedFunction)
[style] 105-105: The function 'constantExpression' is never used.
(unusedFunction)
[style] 109-109: The function 'evaluate' is never used.
(unusedFunction)
[style] 41-41: The function 'truncateConstantValueToExpectedBitwidth' is never used.
(unusedFunction)
[style] 29-29: The function 'modules' is never used.
(unusedFunction)
[style] 33-33: The function 'findModule' is never used.
(unusedFunction)
[style] 248-248: The function 'addThenStatement' is never used.
(unusedFunction)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🇨 Test 🏁 (windows-2022, msvc, Release) / 🏁 windows-2022 msvc Release
- GitHub Check: 🇨 Test 🍎 (macos-14, clang, Release) / 🍎 macos-14 clang Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🇨 Test 🍎 (macos-14, clang, Debug) / 🍎 macos-14 clang Debug
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🐍 Test (macos-14) / 🐍 macos-14
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🐍 Test (windows-2022) / 🐍 windows-2022
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🇨 Coverage / 📈 Coverage
- GitHub Check: 🇨 Lint / 🚨 Lint
🔇 Additional comments (5)
bindings/bindings.cpp (5)
23-42: LGTM!The include structure and namespace setup follow nanobind conventions. The
NOLINT(misc-include-cleaner)comments are appropriate since the nanobind STL headers are required for implicit type conversions even when not explicitly referenced.
169-173: LGTM!The
scoped_estream_redirectimplementation correctly inherits fromscoped_ostream_redirectand defaults tostd::cerr/sys.stderr. Once the parent class issues are addressed, this class will be safe.
179-182: LGTM!The
NB_MODULEentry point andmqt.core.irimport are correctly set up. The import is necessary to ensure the base classqc::QuantumComputationbindings are available before defining derived class bindings.
184-248: LGTM!The class and enum bindings are correctly migrated to nanobind idioms:
rv_policy::reference_internalonqubit_inlining_stack.__getitem__correctly handles lifetime when returning references to internal state (per learnings from the core repository migration).- Enum bindings without
export_values()follow nanobind conventions.nb::overload_castis the correct pattern for overloaded method bindings.def_prop_roanddef_rwusage is appropriate for the respective property types.
250-253: LGTM!The function bindings correctly use
nb::call_guard<scoped_estream_redirect>()to redirect C++ stderr output to Python'ssys.stderr. The comment appropriately notes this is a temporary solution until the synthesizers return errors via return values or output parameters.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
This looks good to me. I pushed a couple of changes addressing CodeRabbit feedback. The solution for the stream isn't particularly pretty but just fine IMHO for this project.
Deleting the copy constructors is a fine solution.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bindings/bindings.cpp`:
- Around line 107-143: The pythonbuf destructor calls _sync() which can invoke
Python (pywrite/pyflush) and throw nb::python_error; wrap the _sync() call
inside ~pythonbuf() in a try/catch block that catches nb::python_error (and
optionally std::exception/... as a fallback) and calls
e.discard_as_unraisable("pythonbuf::~pythonbuf") (or similar location string) to
prevent std::terminate, ensuring any other exceptions are swallowed safely so
destruction never lets exceptions propagate.
## Description After we have switched from `pybind11` to `nanobind` in #514, we can now auto-generate the stub file. ## Checklist: - [x] The pull request only contains commits that are focused and relevant to this change. - [x] ~I have added appropriate tests that cover the new/changed functionality.~ - [x] ~I have updated the documentation to reflect these changes.~ - [x] The changes follow the project's style guidelines and introduce no new warnings. - [x] The changes are fully tested and pass the CI checks. - [x] I have reviewed my own code changes.
2 participants
Description
This PR fully replaces
pybind11withnanobind. This change will allow us to ship Stable ABI wheels, saving us PyPI space.Checklist:
I have added appropriate tests that cover the new/changed functionality.