GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||
GeezerVison is showing github.com — Update `test_itertools.py` to 3.13.7 by ShaharNaveh · Pull Request #6122 · RustPython/RustPython Conversation
|
WalkthroughUpdates in itertools: count() now accepts start and step as keyword or positional arguments. batched() gains a new strict boolean parameter (default false). Internally, PyItertoolsBatched stores a strict flag and next() enforces raising ValueError("batched(): incomplete batch") when strict is true and the final batch is short. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Py as Python Code
participant B as batched(iterable, n, strict)
participant I as PyItertoolsBatched
participant It as Underlying Iterator
Py->>B: construct with iterable, n, strict?
B->>I: py_new(iterable_ref, n, strict)
Note right of I: strict stored in AtomicCell<bool>
loop next()
Py->>I: __next__()
I->>It: collect up to n items
alt collected count == 0
I-->>Py: StopIteration
else collected count in (1..n)
alt strict == true and count != n
I-->>Py: raise ValueError("batched(): incomplete batch")
else
I-->>Py: return batch (len == count)
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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: 0
🧹 Nitpick comments (1)
vm/src/stdlib/itertools.rs (1)
2016-2025: Strict-mode enforcement is correct; minor preallocation nit.Logic correctly raises
ValueError("batched(): incomplete batch")only when the final batch is short and non-empty. Consider preallocating to reduce reallocs:- let mut result: Vec<PyObjectRef> = Vec::new(); + let mut result: Vec<PyObjectRef> = Vec::with_capacity(n);
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Lib/test/test_itertools.pyis excluded by!Lib/**
📒 Files selected for processing (1)
vm/src/stdlib/itertools.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
vm/src/stdlib/itertools.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run tests under miri
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run snippets and cpython tests on wasm-wasi
🔇 Additional comments (5)
vm/src/stdlib/itertools.rs (5)
241-246: Accepting keyword args for count(start, step) looks correct and matches CPython 3.13.This relaxes the arg kind to allow both positional and keyword; good alignment with CPython.
Please confirm pickling behavior for non-default step: CPython includes
stepin__reduce__whenstep != 1. Our current__reduce__only returns(cls, (cur,)), which might dropstepon unpickle. If tests for that exist in 3.13.7, ensure they pass.
1945-1949: Adding strict flag storage in batched is appropriate.Per-instance AtomicCell matches the pattern used elsewhere here.
1957-1959: Keyword-onlystrictwith default False matches CPython signature.Keyword-only via
#[pyarg(named, ...)]is the right choice forbatched(iterable, n, /, *, strict=False).
1966-1971: Argument destructuring updated to include strict — good.
1982-1987: Persisting strict in the instance — good.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
2 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.