Copilot commented on code in PR #7824:
URL: https://github.com/apache/opendal/pull/7824#discussion_r3474649695
##########
bindings/python/src/file.rs:
##########
@@ -194,11 +189,8 @@ impl File {
/// -------
/// int
/// The number of bytes read.
- pub fn readinto(
- &mut self,
- #[gen_stub(override_type(type_repr = "builtins.bytes |
builtins.bytearray", imports=("builtins")))]
- buffer: PyBuffer<u8>,
- ) -> PyResult<usize> {
+ #[pyo3(signature = (buffer: "bytes | bytearray"))]
+ pub fn readinto(&mut self, buffer: PyBuffer<u8>) -> PyResult<usize> {
Review Comment:
`readinto` rejects read-only buffers at runtime (`buffer.readonly()` check)
but the exposed signature/doc currently advertises `bytes | bytearray`, which
implies `bytes` is accepted. This will mislead type-checkers and users; `bytes`
should be removed from the accepted types (a writable buffer like
`bytearray`/`memoryview` is required).
##########
bindings/python/src/file.rs:
##########
@@ -704,23 +669,26 @@ impl AsyncFile {
})
}
- #[gen_stub(override_return_type(type_repr="typing.Self",
imports=("typing")))]
+ // `typing_extensions.Self` (not `typing.Self`) because the binding's
+ // `requires-python` floor is 3.10, where `typing.Self` (3.11+) is absent;
+ // `typing_extensions` back-ports it. Switch to `typing.Self` once the
floor
+ // reaches 3.11.
+ #[pyo3(signature = () -> "typing_extensions.Self")]
fn __aenter__<'a>(slf: PyRef<'a, Self>, py: Python<'a>) ->
PyResult<Bound<'a, PyAny>> {
let slf = slf.into_py_any(py)?;
future_into_py(py, async move { Ok(slf) })
}
Review Comment:
`AsyncFile.__aenter__` returns an awaitable via `future_into_py`, but the
signature annotation currently says it returns `typing_extensions.Self`. For an
async context manager, `__aenter__` must return an `Awaitable[...]`; otherwise
the generated `.pyi` marks it as a synchronous method returning `Self`, which
is incorrect for both runtime calls and type checkers.
##########
bindings/python/src/file.rs:
##########
@@ -790,10 +749,6 @@ impl AsyncFile {
/// -------
/// coroutine
/// An awaitable that returns True if this file is closed.
- #[gen_stub(override_return_type(
- type_repr="collections.abc.Awaitable[builtins.bool]",
- imports=("collections.abc", "builtins")
- ))]
#[getter]
pub fn closed<'p>(&'p self, py: Python<'p>) -> PyResult<Bound<'p, PyAny>> {
let state = self.0.clone();
Review Comment:
`AsyncFile.closed` currently has no `#[pyo3(signature = ...)]` return
annotation even though it returns an awaitable. This causes the generated stub
to fall back to `Any` for `closed`, losing the intended `Awaitable[bool]` type.
Adding an explicit signature keeps stubs accurate without changing runtime
behavior.
##########
bindings/python/justfile:
##########
@@ -55,23 +55,24 @@ clean:
stub-gen: setup
@echo "{{ BOLD }}--- Generating Python type stubs ---{{ NORMAL }}"
@cargo run --quiet --manifest-path=../../dev/Cargo.toml -- generate -l
python
- @cargo run --quiet --bin stub_gen
- @echo "{{ BOLD }}--- Formatting and fixing generated stubs ---{{ NORMAL }}"
- -@bash -c 'shopt -s globstar; uv run ruff check **/*.pyi --fix
--unsafe-fixes --silent || true'
+ @uv run maturin develop --generate-stubs
+ ## This is temporary till the functionalities land in pyo3-introspection.
ref: https://github.com/PyO3/pyo3/issues/5137
+ @echo "{{ BOLD }}--- Post processing generated stubs ---{{ NORMAL }}"
+ @uv run python scripts/postprocess_stubs.py
Review Comment:
`just stub-gen` regenerates stubs that are committed and shipped in release
wheels, so stub generation should use the same Cargo feature set as wheel
builds. The release workflow builds Python wheels with
`--features=...,services-all,...`; without passing `--features services-all`
here, the stub output can drift depending on maturin defaults / local
environment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
