Skip to content

__{get,set,del}item__ without pymethods #6622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
youknowone merged 3 commits into from
Jan 1, 2026
Merged

__{get,set,del}item__ without pymethods #6622

youknowone merged 3 commits into from
Jan 1, 2026

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Jan 1, 2026 •
edited by coderabbitai bot
Loading

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.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 1, 2026 •
edited
Loading

📝 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

Suggested reviewers

  • arihant2math

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
  • 📝 Generate docstrings
📜 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.rs
  • crates/stdlib/src/contextvars.rs
  • crates/stdlib/src/mmap.rs
  • crates/vm/src/builtins/bytearray.rs
  • crates/vm/src/builtins/bytes.rs
  • crates/vm/src/builtins/dict.rs
  • crates/vm/src/builtins/genericalias.rs
  • crates/vm/src/builtins/list.rs
  • crates/vm/src/builtins/mappingproxy.rs
  • crates/vm/src/builtins/memory.rs
  • crates/vm/src/builtins/range.rs
  • crates/vm/src/builtins/str.rs
  • crates/vm/src/builtins/tuple.rs
  • crates/vm/src/builtins/type.rs
  • crates/vm/src/stdlib/collections.rs
  • crates/vm/src/stdlib/ctypes/array.rs
  • crates/vm/src/stdlib/ctypes/pointer.rs
  • crates/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.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 1, 2026

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:

git pull origin slots

@youknowone youknowone changed the title Slots __{get,set,del}item__ without pymethods Jan 1, 2026
@youknowone youknowone marked this pull request as ready for review January 1, 2026 23:08
@youknowone youknowone merged commit 2e29248 into RustPython:main Jan 1, 2026
13 checks passed
@youknowone youknowone deleted the slots branch January 1, 2026 23:29
This was referenced Jan 2, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone