Skip to content

Align itertools constructor binding and conversions - #9632

Merged
youknowone merged 9 commits into
RustPython:mainfrom
youknowdot:fix/itertools-constructor-binding-20261103
Nov 7, 2026
Merged

youknowone merged 9 commits into
RustPython:mainfrom
youknowdot:fix/itertools-constructor-binding-20261103

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Align itertools constructor binding and conversion order with CPython while preserving positional/keyword kinds and subclass keyword handling. Bind tee's optional count before invoking __index__.
  • Build on merged Report a missing argument before converting any #9513: keep typed PyIter arguments for cycle, starmap, takewhile, dropwhile, filterfalse, and pairwise, whose positional arity and keyword checks now happen before conversion. Retain deferred conversion where ordering or source lifetime requires it. In particular, _tee must keep its source alive until copying an existing tee's position.
  • Align batched signed-size conversion, strict truth testing, partial-batch and source-error behavior, and islice start/stop/step conversion ordering and ValueErrors.
  • Validate combinations sizes before consuming the iterable, preserve permutations' distinct pool-first/PyLong-only semantics, and distinguish an omitted repeat count from explicit None. Share the C-ssize conversion for relevant counts and retain user conversion exceptions.
  • Add focused regression coverage for argument rejection before __iter__, direct __new__ calls, subclass keywords, and _tee source finalization. Resolve the related Clippy warnings.

Validation

  • cargo test --locked -p rustpython-vm --lib: 170 passed.
  • Threading-enabled RustPython debug build with stdlib,stdio,importlib,host_env,doc,threading; rustpython -m test test_itertools: SUCCESS, 137 run, 22 skipped.
  • pytest -v test_snippets.py -k stdlib_itertools: 2 passed (RustPython and CPython 3.12.14).
  • Targeted VM Clippy with compiler,gc,host_env,stdio,doc,threading,importlib,encodings: passed without warnings.
  • Normal commit hooks, including rustfmt, cspell, and Ruff: passed.
  • Full workspace/C-API suites and release-mode tests have not completed. The first non-threading build failed the concurrent tee test; rerunning the full module with threading enabled passed.

This does not claim complete constructor parity. Existing arity-versus-keyword diagnostic precedence, other allocation and argument/source lifetime differences, try_index normalization details, and broader concurrency behavior remain outside scope. Deferred conversion retains the original argument during the constructor call and adds bounded reference-management work; no benchmark was run. zip_longest variadic binding remains unchanged because deferring it requires a separately assessed temporary allocation.

AI assistance

OpenAI Codex assisted with implementation, source review, regression tests, and local validation.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Caution

Review failed

The pull request is closed.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Assisted-by: OpenAI Codex:model version not exposed
Bind the two required and two optional raw positional arguments together. Preserve explicit None values and deferred index conversion while removing the per-arity parsing branches.

Assisted-by: OpenAI Codex:model version not exposed
Use PyIter for cycle, starmap, takewhile, dropwhile, filterfalse, and pairwise now that positional arity and keyword checks precede conversion. Keep _tee source ownership through its position snapshot, and cover constructor callbacks, subclass keywords, and source finalization.

Use an islice step expression and gate the remaining int value helper to its test, serde, and JIT consumers to keep Clippy clean.

Assisted-by: OpenAI Codex:model version not exposed
@youknowdot
youknowdot force-pushed the fix/itertools-constructor-binding-20261103 branch from c756f0d to 60e3d91 Compare November 6, 2026 03:01
struct CompressNewArgs {
#[pyarg(any)]
data: PyIter,
data: PyObjectRef,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check this change is necessary after #9563

@youknowone
youknowone marked this pull request as ready for review November 7, 2026 11:57
@youknowone
youknowone merged commit 171879e into RustPython:main Nov 7, 2026
20 checks passed
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-07T11:57:57.072635Z
We couldn’t complete this review. Comment @codex review to try again.
60e3d91 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants