| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
issue : RustPython#3609 Signed-off-by: Yash Suthar <yashsuthar983@gmail.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists. You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file. WalkthroughA new anext builtin function is added to the RustPython virtual machine's builtins module. The function accepts an asynchronous iterator and an optional default value, calling the iterator's __anext__ method to retrieve an awaitable. Default value handling is incomplete and marked with a TODO comment. Changes
Sequence DiagramsequenceDiagram
participant User as User Code
participant Builtin as anext() Builtin
participant Iterator as Async Iterator
participant Awaitable as __anext__ Awaitable
User->>Builtin: anext(aiter, [default_value])
Builtin->>Iterator: Access __anext__ method
Iterator->>Awaitable: Return awaitable
Awaitable-->>Builtin: Awaitable object
alt default_value provided
Note over Builtin: TODO: Handle default value<br/>(incomplete)
end
Builtin-->>User: Return awaitable
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
|
@youknowone can you provide some direction to implement PyAnextAwaitable. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review detailsConfiguration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between fda9cee and d08b04e.
📒 Files selected for processing (1)📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management
Files:
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use RustPython macros (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust
Files:
Sorry, something went wrong.
| #[pyfunction] | ||
| fn anext( | ||
| aiter: PyObjectRef, | ||
| default_value: OptionalArg<PyObjectRef>, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let awaitable = vm.call_method(&aiter, "__anext__", ())?; | ||
|
|
||
| if default_value.is_missing() { | ||
| Ok(awaitable) | ||
| } else { | ||
| // TODO: Implement CPython like PyAnextAwaitable to properly handle the default value. | ||
| Ok(awaitable) | ||
| } | ||
| } |
There was a problem hiding this comment.
⚠️ Potential issue | 🟠 Major
Incomplete default value handling makes the function only partially functional.
The default_value parameter is accepted but completely ignored—both branches return the same awaitable regardless of whether a default is provided. This causes the function to fail the second use case described in the PR objectives: when the async iterator is exhausted and a default value is provided, RustPython raises StopAsyncIteration instead of returning the default like CPython does.
While the TODO comment acknowledges this limitation, having a function signature that accepts a parameter it cannot honor is problematic because:
Consider one of these approaches before merging:
Option 1 (preferred if feasible): Complete the implementation by adding PyAnextAwaitable to wrap the awaitable and handle the default value case.
Option 2: Reject the default parameter explicitly until support is implemented:
#[pyfunction]
fn anext(
aiter: PyObjectRef,
default_value: OptionalArg<PyObjectRef>,
vm: &VirtualMachine,
) -> PyResult {
let awaitable = vm.call_method(&aiter, "__anext__", ())?;
- if default_value.is_missing() {
- Ok(awaitable)
- } else {
- // TODO: Implement CPython like PyAnextAwaitable to properly handle the default value.
- Ok(awaitable)
+ if let OptionalArg::Present(_) = default_value {
+ Err(vm.new_not_implemented_error(
+ "anext() with default value is not yet implemented".to_owned()
+ ))
+ } else {
+ Ok(awaitable)
}
}Option 3: Defer merging this PR until the default value handling is fully implemented.
📝 Committable suggestion‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[pyfunction] | |
| fn anext( | |
| aiter: PyObjectRef, | |
| default_value: OptionalArg<PyObjectRef>, | |
| vm: &VirtualMachine, | |
| ) -> PyResult { | |
| let awaitable = vm.call_method(&aiter, "__anext__", ())?; | |
| if default_value.is_missing() { | |
| Ok(awaitable) | |
| } else { | |
| // TODO: Implement CPython like PyAnextAwaitable to properly handle the default value. | |
| Ok(awaitable) | |
| } | |
| } | |
| #[pyfunction] | |
| fn anext( | |
| aiter: PyObjectRef, | |
| default_value: OptionalArg<PyObjectRef>, | |
| vm: &VirtualMachine, | |
| ) -> PyResult { | |
| let awaitable = vm.call_method(&aiter, "__anext__", ())?; | |
| if let OptionalArg::Present(_) = default_value { | |
| Err(vm.new_not_implemented_error( | |
| "anext() with default value is not yet implemented".to_owned() | |
| )) | |
| } else { | |
| Ok(awaitable) | |
| } | |
| } |
Sorry, something went wrong.
|
Thanks! By looking the CI result, the test is failing because this patch fixed a few tests: ====================================================================== UNEXPECTED SUCCESS: test_anext_await_raises (test.test_asyncgen.AsyncGenAsyncioTest.test_anext_await_raises) UNEXPECTED SUCCESS: test_anext_return_generator (test.test_asyncgen.AsyncGenAsyncioTest.test_anext_return_generator) UNEXPECTED SUCCESS: test_anext_return_iterator (test.test_asyncgen.AsyncGenAsyncioTest.test_anext_return_iterator) ---------------------------------------------------------------------- Remove @expectedFailure from those tests will fix the CI. For PyAnextAwaitable, I'd start to defining the type on vm/src/builtins/iter.rs using #[pyclass]. Searching #[pyclass in source code will give some idea about it. Please poke me when you are blocked by something. |
Sorry, something went wrong.
Signed-off-by: Yash Suthar <yashsuthar983@gmail.com>
Signed-off-by: Yash Suthar <yashsuthar983@gmail.com>
There was a problem hiding this comment.
👍
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
issue : #3609
Currently passes
Failed case ,need PyAnextAwaitable
Summary by CodeRabbit