Repository navigation
Move PyList Python methods and operators to Py<PyList> - #8848
youknowone wants to merge 3 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ❓ Inconclusive | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 114 functions across 22 files. (1 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation 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. Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled. Title check ✅ Passed The title clearly and concisely describes the primary change: moving PyList Python methods and operators to Py.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 114 functions across 22 files. (1 skipped: 1 too large.)
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- 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.
ℹ️ Recent review info
-
⚙️ Run configuration
-
Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: 0471d9e0-20a5-45ec-a3e2-cb03be74a30a
📒 Files selected for processing (20)
-
crates/capi/src/listobject.rs
crates/derive-impl/src/pyclass.rs
crates/derive-impl/src/pystructseq.rs
crates/stdlib/src/elementtree.rs
crates/vm/src/builtins/dict.rs
crates/vm/src/builtins/function.rs
crates/vm/src/builtins/list.rs
crates/vm/src/builtins/slice.rs
crates/vm/src/builtins/str.rs
crates/vm/src/builtins/tuple.rs
crates/vm/src/builtins/type.rs
crates/vm/src/exceptions.rs
crates/vm/src/frame.rs
crates/vm/src/object/core.rs
crates/vm/src/object/traverse.rs
crates/vm/src/stdlib/_collections.rs
crates/vm/src/stdlib/_collections/ordered_dict.rs
crates/vm/src/stdlib/_functools.rs
crates/vm/src/stdlib/_testinternalcapi.rs
crates/vm/src/stdlib/_thread.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
ℹ️ Recent review info
-
⚙️ Run configuration
-
Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID:
0471d9e0-20a5-45ec-a3e2-cb03be74a30a
📒 Files selected for processing (20)
-
crates/capi/src/listobject.rscrates/derive-impl/src/pyclass.rscrates/derive-impl/src/pystructseq.rscrates/stdlib/src/elementtree.rscrates/vm/src/builtins/dict.rscrates/vm/src/builtins/function.rscrates/vm/src/builtins/list.rscrates/vm/src/builtins/slice.rscrates/vm/src/builtins/str.rscrates/vm/src/builtins/tuple.rscrates/vm/src/builtins/type.rscrates/vm/src/exceptions.rscrates/vm/src/frame.rscrates/vm/src/object/core.rscrates/vm/src/object/traverse.rscrates/vm/src/stdlib/_collections.rscrates/vm/src/stdlib/_collections/ordered_dict.rscrates/vm/src/stdlib/_functools.rscrates/vm/src/stdlib/_testinternalcapi.rscrates/vm/src/stdlib/_thread.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
-
e82b749 to
e758e5b
Compare
a90cbe6 to
d4112a2
Compare
Keep list operations on Py<PyList> and route the C API through them. Reuse current checked slice assignment and finalizer ordering, retaining the Traverse method rename so list.clear() is not shadowed. Keep replaced items alive through the write guard scope without a redundant terminal drop. Assisted-by: Claude Code:claude-opus-5-5 Assisted-by: Codex:model-version-unavailable
Assisted-by: Codex:model-version-unavailable
ca56b94 to
eb35587
Compare
Call the RustPython C-API implementation directly so the negative-index regression test also compiles when workspace Clippy selects pre-3.13 PyO3 bindings. Keep the null result and IndexError assertions intact. Assisted-by: Codex:model-version-unavailable
Follow-up to #8819, same rules applied to
list. Four commits.Move PyList Python methods and operators to Py
Py<PyList>::borrow_vec()/borrow_vec_mut()are the views.PyList::__len__()is removed; its callers readborrow_vec().len().The Python interface of
listmoves from thePyListpayload to a#[pyclass] impl Py<PyList>block, registered throughwith(Py, ...):#[pymethod](append,extend,insert,clear,copy,__sizeof__,reverse,__reversed__,__getitem__,count,index,pop,remove,sort) and#[pyclassmethod] __class_getitem____add__(which replacesconcat()),__iadd__,__mul__,__imul__,__setitem__,__delitem__,__contains__andinplace_concatThe
#[pyclass] impl PyListblock keeps onlywith(...)andflags(...).PyListkeeps the helpers that work on the payload:_getitem,_setitem,_delitem,repeat,irepeatandsort, whichsorted()anddir()also call on aPyListvalue before it becomes an object.Py<PyList>reaches them through thepayloadfield. The mapping and sequence slots call thePy<PyList>operators.GenericAlias.__dir__works on thePyListvalue returned byvm.dir(), so it appends throughborrow_vec_mut()and checks membership withmut_contains().Derefis unchanged and behavior is unchanged.Rename Traverse::clear to clear_refs
Traverse::clearbecomesclear_refs, andMaybeTraverse::try_clearbecomestry_clear_refs. Before this,list.clear()on aPyRef<PyList>resolved to theTraversetrait method instead ofPy<PyList>::clear. The derive macros and every existing impl are renamed.try_clear_obj,HAS_CLEARand#[pyclass(clear)]are unchanged.Expose the PyList_* operations on Py
Only the operations that have a
PyList_*counterpart are pub onPy<PyList>. Names follow pyo3'sPyListMethods.Py<PyList>len(),is_empty()PyList_Sizeget_item(index, vm)PyList_GetItemRefset_item(index, item, vm)PyList_SetItemget_slice(low, high, vm)PyList_GetSliceset_slice(low, high, items, vm)PyList_SetSlicedel_slice(low, high)PyList_SetSlicewithNULLappend,extend,insert,clear,reverse(now pub)PyList_Append,PyList_Extend,PyList_Insert,PyList_Clear,PyList_Reversesort(vm)PyList_Sortto_tuple(vm)PyList_AsTuplesortpymethod is now implemented bysort_with_options().get_slice,set_sliceanddel_sliceclamp both bounds to the list.set_item,set_sliceanddel_slicedrop the replaced elements after the list lock is released.PyList_Size,PyList_Append,PyList_Insert,PyList_ReverseandPyList_AsTuplecall these methods.Call the Py methods from the remaining capi PyList_* functions
PyList_GetItemRef,PyList_SetItem,PyList_GetSlice,PyList_SetSliceandPyList_Sortnow call thePy<PyList>methods. Their behavior changes to match the C API:PyList_GetSliceandPyList_SetSliceclamp negative bounds to 0 instead of counting them from the end of the list.PyList_SetSlicecollects the new items before taking the list lock, so a list can be assigned into itself.PyList_GetItemRefandPyList_SetItemraiseIndexErrorwithout the index in the message.PyList_Sortsorts the list directly instead of calling itssortmethod.PyList_SetItemstill appends when the index equals the length and the list has spare capacity, as afterPyList_New.New capi tests cover negative bounds, deleting a slice, and assigning a list into itself.
🤖 Generated with Claude Code
Summary by CodeRabbit