-
Notifications
You must be signed in to change notification settings - Fork 707
feat(bindings/python)!: Add stubs for remaining types #6720
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
Conversation
| import typing | ||
|
|
||
| PathBuf: TypeAlias = str | os.PathLike | ||
| from opendal import capability, exceptions, file, layers, types |
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.
this import is auto generated and capability here was conflicting with capability method on Operator and AsyncOperator.
So had to change method name to full_capability - a user facing change.
| ConcurrentLimitLayer. | ||
| Create a layer that limits the number of concurrent operations. | ||
| A layer that limits the number of concurrent operations. |
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.
minor wording changes, to de duplicate heading in mkdocs
| /// Layer | ||
| /// | ||
| /// Layers are used to intercept the operations on the underlying storage. |
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.
minor wording changes, to de duplicate heading in mkdocs
| m.add_class::<Operator>()?; | ||
| m.add_class::<AsyncOperator>()?; |
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.
initially i wanted to have operators in in their own py module, but then this would require a major breaking change from from opendal import Operator to from opendal.operator import Operator.
I tried messing with the __init__.py to get around this, but maybe this can be taken up in another PR.
| #[gen_stub(override_return_type( | ||
| type_repr="collections.abc.Awaitable[builtins.int]", | ||
| imports=("collections.abc", "builtins") | ||
| ))] |
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.
the generated functions are not async on py side, but the return types have been overridden to Awaitable as advised here.
same has been done on operator side
|
|
||
| [lint.per-file-ignores] | ||
| "*.pyi" = ["PYI021", "ANN003", "RUF100"] | ||
| "*.pyi" = ["PYI021", "ANN003", "RUF100", "ANN401"] |
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.
necessary for the new/ open on Operator and AsyncOperator which take **kwargs.
| cap = operator.full_capability() | ||
| assert cap is not None | ||
| assert cap.read is not None | ||
|
|
||
|
|
||
| def test_capability_exception(service_name, operator): | ||
| cap = operator.capability() | ||
| cap = operator.full_capability() |
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.
see this comment for explanation.
|
@Xuanwo final one for stubs for your review, whenever you're free. Copilot Summary:This pull request refines the Python bindings for OpenDAL, focusing on improved documentation, more accurate type stubs, and consistent module organization. The most significant changes include enhanced docstrings and type annotations for the Python API improvements
New types and documentation
Layer class docstring improvements
Configuration and dependency updates
|
Xuanwo
left a comment
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.
Perfect, thank you for working on this!
Which issue does this PR close?
Parts from #6622
Rationale for this change
Closes #6253
What changes are included in this PR?
Are there any user-facing changes?
Yes.
capability()on Operator and AsyncOperator has been renamed tofull_capability()