Skip to content

Join lists and tuples directly in str.join - #8826

Merged
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:str_join_lists_tuples
Sep 26, 2026
Merged

Join lists and tuples directly in str.join#8826
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:str_join_lists_tuples

Conversation

@1ndahous3

@1ndahous3 1ndahous3 commented Sep 26, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

str.join always went through the iterator protocol: it created an iterator, looked up __length_hint__ (allocating a bound method), and grew the result string item by item. For an exact list or tuple it now works on the items directly, as CPython's PySequence_Fast does. It checks the items and sums their lengths, then builds the result in a single allocation. Other iterables keep the existing path. The list's read lock is held for the duration; no Python code runs while it's held.

Performance

Instructions per call:

sep.join(...) before after CPython
3-item list 4,880 2,061 660
20-item list 10,099 4,275 2,247
3-item tuple 4,775 2,071 662
1-item list 3,694 1,308 222
generator (unchanged path) 14,960 15,130 3,564

Wall time: ','.join(row) over 50,000 CSV-style rows 35.3 → 10.8 ms; ''.join(parts) of 200 chunks ×2,000 16.8 → 2.5 ms (CPython: 2.0 ms).

AI assistance

Written with Claude Code (claude-opus-5-5): profiling, implementation, and testing were AI-assisted end to end, reviewed by a human before submission.

Summary by CodeRabbit

  • Bug Fixes
    • Improved str.join behavior for lists, tuples, empty iterables, and generators.
    • Non-string elements now raise TypeError with the expected message.
    • Joining a single string preserves its identity, while joining string subclasses returns an exact str.

@1ndahous3
Join lists and tuples directly in str.join
463d1e6 
Assisted-by: Claude Code:claude-opus-5-5
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 26, 2026

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 81211b57-93b7-4942-a899-c6999b541524

📥 Commits

Reviewing files that changed from the base of the PR and between f3adc69 and 463d1e6.

📒 Files selected for processing (2)
  • crates/vm/src/builtins/str.rs
  • extra_tests/snippets/builtin_str.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Walkthrough

str.join now handles exact lists and tuples with a direct sequence path. Other iterables continue through the iterator-based path. Tests cover empty and single-item inputs, generators, non-string elements, and string subclasses.

Changes

String Join Sequence Handling

Layer / File(s) Summary
Direct sequence join handling
crates/vm/src/builtins/str.rs, extra_tests/snippets/builtin_str.py		 Exact lists and tuples use join_items, which validates elements, computes capacity, and constructs the joined string. Tests cover tuple, empty-list, and generator inputs, indexed TypeError reporting, subclass results, and single-item identity.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 463d1

No actionable issue is established for the new list and tuple join path; the change is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: direct handling of lists and tuples in str.join.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

Comment @coderabbitai help to get the list of available commands.

youknowone
youknowone approved these changes Sep 26, 2026
View reviewed changes

@youknowone youknowone left a comment

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.

👍

@youknowone
youknowone merged commit 206b090 into RustPython:main Sep 26, 2026
21 checks passed
@1ndahous3
1ndahous3 deleted the str_join_lists_tuples branch September 26, 2026 20:28
@1ndahous3 1ndahous3 mentioned this pull request Sep 28, 2026
Merged
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@1ndahous3 @youknowone