Conversation
denialhaag
added
python
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughReplace legacy pybind11 binding files with new nanobind Changes
Sequence Diagram(s)(Skipped) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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. 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: 2
🤖 Fix all issues with AI agents
In `@bindings/register_application_scheme.cpp`:
- Line 21: Rename the registration function to match the enum name: change
registerApplicationSchema to registerApplicationScheme (note "Scheme" not
"Schema") in the function definition and its forward declaration, and update all
call sites that reference registerApplicationSchema to use
registerApplicationScheme so the symbol matches the enum ApplicationScheme.
In `@noxfile.py`:
- Around line 204-237: Before running nanobind.stubgen, remove any previously
generated stubs under package_root to avoid stale .pyi files; locate the
package_root and pattern_file variables and the session.run call that invokes
"python -m nanobind.stubgen", then add a cleanup step that deletes existing .pyi
files or the target directory (e.g., remove Path(package_root)/*.pyi or the
whole generated tree) prior to the stubgen session.run so the subsequent listing
into pyi_files reflects only newly generated stubs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bindings/register_equivalence_checking_manager.cpp`:
- Around line 11-20: The code calls nlohmann::json::parse (used in
EquivalenceCheckingManager code) but does not include the nlohmann/json header;
add the missing include for nlohmann/json.hpp near the other includes (alongside
Configuration.hpp, EquivalenceCheckingManager.hpp, etc.) so the call to
nlohmann::json::parse is not relying on fragile transitive includes and will
compile reliably.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@noxfile.py`:
- Around line 221-225: The code currently calls session.warn("No .pyi files
found") and returns when pyi_files is empty, which lets the session exit
successfully; change this to fail the session by replacing the warn+return with
a hard failure (e.g., call session.error("No .pyi files found") or raise
SystemExit/RuntimeError) so missing stubs cause a nonzero exit; update the block
that checks pyi_files (the variables pyi_files and package_root and the session
call) to invoke session.error with a clear message instead of session.warn and
remove the silent return.
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
This looks great. Just noticed one small inconsistency.
2 participants
Description
After we have switched from
pybind11tonanobindin #817, we can now auto-generate the stub file. This PR defines a correspondingnoxsession and copies over all existing docstrings to the bindings code.Checklist:
I have added appropriate tests that cover the new/changed functionality.I have updated the documentation to reflect these changes.