-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
build: add python3.13 support #103
Conversation
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
54d7cc8
to
ec7b3c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bindings/python/CMakeLists.txt (1)
188-188
: Consider adding test requirements for Python 3.13Since this adds support for a new Python version, ensure that:
- CI pipelines are updated to test Python 3.13 builds
- Test coverage includes Python 3.13-specific scenarios
Would you like help in setting up test requirements or CI configurations for Python 3.13?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bindings/python/CMakeLists.txt
(1 hunks)bindings/python/requirements.txt
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- bindings/python/requirements.txt
@@ -185,7 +185,7 @@ FIND_PACKAGE (pybind11 2.12.0 REQUIRED) | |||
### Python | |||
|
|||
# Python versions to build for and support | |||
SET (PYTHON_SEARCH_VERSIONS 3.9 3.10 3.11 3.12 CACHE STRING "Versions of Python bindings to build.") | |||
SET (PYTHON_SEARCH_VERSIONS 3.9 3.10 3.11 3.12 3.13 CACHE STRING "Versions of Python bindings to build.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Based on the search results and the earlier GitHub issue search that revealed an open issue "Python v3.13 related things to look into" (#4870), I can now generate the final response:
Python 3.13 support needs further investigation
The addition of Python 3.13 to PYTHON_SEARCH_VERSIONS
may be premature as:
- There's an open issue in pybind11 (#4870) specifically about Python 3.13 compatibility
- Python 3.13 support is still being investigated by the pybind11 team
- The CMake configuration will safely skip Python 3.13 if it's not found, but enabling it now might create expectations of support
Recommendation:
- Consider waiting for official Python 3.13 support in pybind11 before adding it to
PYTHON_SEARCH_VERSIONS
- Alternatively, add a comment indicating that Python 3.13 support is experimental
🔗 Analysis chain
Verify pybind11 compatibility with Python 3.13
The addition of Python 3.13 support aligns with the PR objective. However, since Python 3.13 is relatively new, please ensure:
- The project's pybind11 version (2.12.0) supports Python 3.13
- All binding features work correctly with Python 3.13
Let's verify pybind11's Python 3.13 support:
Let me try another approach to verify pybind11's Python 3.13 support by checking their source code directly:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check pybind11's Python version support in their repository
# Check if pybind11 2.12.0 mentions Python 3.13 support in their changelog
gh api repos/pybind/pybind11/contents/docs/changelog.rst | jq -r '.content' | base64 -d | grep -A 5 "v2.12.0"
# Check for any open/closed issues related to Python 3.13 support
gh api graphql -f query='
{
search(query: "repo:pybind/pybind11 Python 3.13", type: ISSUE, first: 5) {
nodes {
... on Issue {
title
state
url
}
}
}
}'
Length of output: 982
Script:
#!/bin/bash
# Check pybind11's Python version support in their CMake configuration
fd CMakeLists.txt | xargs rg -A 5 "PYTHON_VERSION|Python version"
# Check if there are any Python version checks or limitations in pybind11 headers
fd "\.h$" | xargs rg -A 5 "PY_VERSION|Python 3.13"
# Look for any Python version related tests
fd "test" | xargs rg -A 5 "Python 3.13|py3.13"
Length of output: 7903
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #103 +/- ##
=======================================
Coverage 69.47% 69.47%
=======================================
Files 7 7
Lines 498 498
=======================================
Hits 346 346
Misses 152 152 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
Bug Fixes
open-space-toolkit-core
dependency to allow for improvements in version 4.1.