GeezerVison is showing github.com — __{get,set,del}item__ without pymethods by youknowone · Pull Request #6622 · RustPython/RustPython
Conversation
youknowone
commented
Jan 1, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary by CodeRabbit
-
Breaking Changes
- Subscript access operations (
[], assignment, deletion) are no longer available for arrays, byte arrays, bytes, dictionaries, lists, tuples, ranges, strings, deques, memory views, and ctypes objects.
-
Bug Fixes
- Improved deterministic ordering of type slot initialization.
✏️ Tip: You can customize this high-level summary in your review settings.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Jan 1, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
📝 Walkthrough
Walkthrough
This PR systematically removes #[pymethod] attribute annotations from __getitem__, __setitem__, and __delitem__ methods across 16+ builtin and stdlib modules, reducing their Python-level exposure while preserving Rust implementations. Additionally, a deterministic sorting step is added to slot name processing in type.rs.
Changes
| Cohort / File(s) |
Summary |
Core Builtins (Array/Sequence Types)
crates/vm/src/builtins/list.rs, crates/vm/src/builtins/tuple.rs, crates/vm/src/builtins/range.rs, crates/vm/src/builtins/str.rs |
Removed #[pymethod] from __getitem__, __setitem__, and __delitem__ methods, reducing Python method exposure for sequence item access operations. |
Core Builtins (Container/Mapping Types)
crates/vm/src/builtins/dict.rs, crates/vm/src/builtins/bytes.rs, crates/vm/src/builtins/bytearray.rs |
Removed #[pymethod] from __getitem__, __setitem__, and __delitem__ methods on mapping/buffer-like types, altering Python-level subscript and item assignment access. |
Advanced Builtins
crates/vm/src/builtins/genericalias.rs, crates/vm/src/builtins/mappingproxy.rs, crates/vm/src/builtins/memory.rs |
Removed #[pymethod] from __getitem__ and related item operations; for memory.rs also includes __delitem__, __setitem__, and __class_getitem__. |
Stdlib Collections
crates/stdlib/src/array.rs, crates/stdlib/src/contextvars.rs, crates/stdlib/src/mmap.rs, crates/vm/src/stdlib/collections.rs, crates/vm/src/stdlib/sre.rs |
Removed #[pymethod] from item access methods (getitem, setitem, delitem); exposure to Python subscript operations is reduced. |
Ctypes Modules
crates/vm/src/stdlib/ctypes/array.rs, crates/vm/src/stdlib/ctypes/pointer.rs |
Removed #[pymethod] from array and pointer subscript/assignment methods (getitem, setitem, delitem). |
Slot Processing
crates/vm/src/builtins/type.rs |
Added deterministic sorting of slot names by collecting slot_name_set into a sorted Vec before iteration in PyType::init_slots to ensure stable processing order. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- More slot impl #6618: Adjusts slot wiring and handling machinery; directly related to how item/subscript slot exposure is managed after these pymethod removals.
- sequence, mapping slots and fix separate __delitem__ slots #6621: Overhauls sequence/mapping slot machinery and AsMapping hookups; pairs with these changes to re-route item operations through alternate dispatch paths.
- ctypes overhaul #6450: Related ctypes overhaul affecting the same array and pointer
__getitem__/__setitem__/__delitem__ functions in the ctypes modules.
Suggested reviewers
Poem
🐰 Methods hop away from Python's sight,
No pymethod decorators in flight!
Slots are sorted, deterministic and true,
The bindings now route through mechanisms new. ✨
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name |
Status |
Explanation |
Resolution |
| Docstring Coverage |
⚠️ Warning |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. |
You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name |
Status |
Explanation |
| Description Check |
✅ Passed |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check |
✅ Passed |
The title accurately describes the main change: removing #[pymethod] attributes from getitem, setitem, and delitem methods across multiple types throughout the codebase. |
✨ Finishing touches
📜 Recent review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 1f8ef0a and 6064ab4.
📒 Files selected for processing (18)
crates/stdlib/src/array.rscrates/stdlib/src/contextvars.rscrates/stdlib/src/mmap.rscrates/vm/src/builtins/bytearray.rscrates/vm/src/builtins/bytes.rscrates/vm/src/builtins/dict.rscrates/vm/src/builtins/genericalias.rscrates/vm/src/builtins/list.rscrates/vm/src/builtins/mappingproxy.rscrates/vm/src/builtins/memory.rscrates/vm/src/builtins/range.rscrates/vm/src/builtins/str.rscrates/vm/src/builtins/tuple.rscrates/vm/src/builtins/type.rscrates/vm/src/stdlib/collections.rscrates/vm/src/stdlib/ctypes/array.rscrates/vm/src/stdlib/ctypes/pointer.rscrates/vm/src/stdlib/sre.rs
💤 Files with no reviewable changes (17)
- crates/vm/src/builtins/mappingproxy.rs
- crates/vm/src/builtins/range.rs
- crates/vm/src/builtins/list.rs
- crates/vm/src/builtins/genericalias.rs
- crates/stdlib/src/mmap.rs
- crates/vm/src/builtins/tuple.rs
- crates/vm/src/stdlib/sre.rs
- crates/vm/src/stdlib/collections.rs
- crates/vm/src/builtins/bytes.rs
- crates/stdlib/src/array.rs
- crates/vm/src/stdlib/ctypes/pointer.rs
- crates/vm/src/builtins/str.rs
- crates/vm/src/stdlib/ctypes/array.rs
- crates/stdlib/src/contextvars.rs
- crates/vm/src/builtins/memory.rs
- crates/vm/src/builtins/dict.rs
- crates/vm/src/builtins/bytearray.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by 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:
crates/vm/src/builtins/type.rs
🔇 Additional comments (1)
crates/vm/src/builtins/type.rs (1)
442-445: LGTM! Deterministic slot processing order.
This change improves reproducibility by sorting slot names alphabetically before processing. Since HashSet iteration order is non-deterministic, this ensures consistent behavior across runs, which is valuable for testing and debugging.
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 and usage tips.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Code has been automatically formatted
The code in this PR has been formatted using cargo fmt --all.
Please pull the latest changes before pushing again:
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jan 2, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|