-
Notifications
You must be signed in to change notification settings - Fork 710
refactor(bindings/python): Update Stubs and dependencies for split #7181
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR refactors the Python bindings for OpenDAL by renaming internal Rust structs with a Py prefix and removing the services module. The changes prepare the codebase for a potential split or reorganization by simplifying the API surface.
Changes:
- Renamed Rust types (
Operator→PyOperator,AsyncOperator→PyAsyncOperator,Capability→PyCapability) using thenameattribute in#[pyclass]to maintain the same Python-facing API - Removed the
servicesmodule andSchemeenum, simplifying operator constructors to accept only string schemes - Updated stub files with proper
__all__exports and fixed type annotations (adding| Noneto optional datetime parameters) - Updated dependencies: bumped
pyo3-stub-gento 0.18, relaxed version constraints on other dependencies - Modified build configuration in
pyproject.tomland updated justfile scripts
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
bindings/python/src/operator.rs |
Renamed structs to PyOperator/PyAsyncOperator, simplified constructors, added deprecation to scan() methods |
bindings/python/src/capability.rs |
Renamed Capability to PyCapability with proper Python name mapping |
bindings/python/src/lib.rs |
Removed services module, updated struct references, added #[allow(deprecated)] |
bindings/python/python/opendal/__init__.py |
Changed import from _opendal to _core, removed services from exports |
bindings/python/python/opendal/__init__.pyi |
Added new stub file with module exports |
bindings/python/python/opendal/services.pyi |
Deleted entire file (87 lines) containing Scheme enum |
bindings/python/python/opendal/operator/__init__.pyi |
New stub file replacing old operator.pyi, simplified constructor signatures |
bindings/python/python/opendal/types/__init__.pyi |
Added __all__ exports and updated ruff noqa |
bindings/python/python/opendal/layers/__init__.pyi |
Added __all__ exports |
bindings/python/python/opendal/file/__init__.pyi |
Added __all__ exports |
bindings/python/python/opendal/exceptions/__init__.pyi |
Added __all__ exports |
bindings/python/python/opendal/capability/__init__.pyi |
Added __all__ exports |
bindings/python/Cargo.toml |
Updated dependencies with relaxed version constraints, bumped pyo3-stub-gen |
bindings/python/pyproject.toml |
Removed maturin features/strip, merged lint into dev, updated dependencies |
bindings/python/justfile |
Improved build scripts with better formatting and cleanup logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @echo "{{ BOLD }}--- Removing build directories, other caches, python bytecode and compiled extensions ---{{ NORMAL }}" | ||
| @find . \ | ||
| \( -type d \( -name __pycache__ -o -name .venv -o -name .build -o -name dist -o -name .pytest_cache -o -name .mypy_cache -o -name .hypothesis -o -name .ruff_cache \) -prune -exec rm -rf {} + \) \ |
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.
same logic as before, just done in 1 command now
|
|
||
| from opendal.operator import AsyncOperator, Operator # pyright:ignore | ||
| from opendal._opendal import * # ty: ignore | ||
| from opendal.operator import AsyncOperator, Operator |
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.
@frostming as you made some recent changes, can you check this out, if this works ?
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.
No, it reverts my change and the typecheckers can't find types, because opendal._opendal is not typed.
In fact I don't understand why you moved *.pyi into its own package. it looks less clean
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.
In fact I don't understand why you moved *.pyi into its own package. it looks less clean
This is done by a recent update of pyo3-stub-gen that is used to generate stubs. the inclusion of all the entries in __all__ is actually working much better for me with both ty and pyright in zed
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.
9841388 to
c7203c2
Compare

Which issue does this PR close?
Closes #.
Links to: #6748
Rationale for this change
This PR refactors the Python bindings for OpenDAL by renaming internal Rust structs with a
Pyprefix and removing theservicesmodule.The changes prepare the py bindings codebase for a future split/ reorganization by simplifying the API surface.
What changes are included in this PR?
Operator→PyOperator,AsyncOperator→PyAsyncOperator,Capability→PyCapability) using thenameattribute in#[pyclass]to maintain the same Python-facing API. this was done to avoid confusion withcoretypes.servicesmodule andSchemeenum, simplifying operator constructors to accept only string schemes in line withcore__all__exports and fixed type annotations (adding| Noneto optional parameters). this is becausepyo3-stub-gen>=0.18uses a different logic to render stubs.pyo3-stub-gento 0.18, relaxed version constraints on other dependenciespyproject.tomland updated justfile scripts__init__.pyfor better discoveryAre there any user-facing changes?
Yes.
Following breaking changes have been introduced:
servicemodule is removed from rust and py side for these bindingslintextra group has been removed, and its dependencies (ruff) have been moved to thedevgroup. This is a breaking change for users who may have been usingpip install opendal[lint].AI Usage Statement
None