Upgrade typing to 3.13.5 by youknowone · Pull Request #5850 · RustPython/RustPython

coderabbitai

youknowone commented
Jun 27, 2025
•

edited by coderabbitai Bot

Loading

Summary by CodeRabbit

New Features
- Introduced an override decorator to mark methods as overrides at runtime.
- Improved error reporting for protocols with non-method members when using issubclass().
Documentation
- Expanded docstrings for several typing utilities with new examples and usage guidance.
Refactor
- Refactored internal instance checking logic for better alignment with expected Python behavior.
Tests
- Updated test metadata by removing expected failure markers from certain typing tests.

Walkthrough

This change refactors the is_instance and real_is_instance logic in the RustPython VM to separate core and recursive isinstance checks, updates test metadata in test_typing.py by removing expected failure decorators, and enhances the typing module with expanded docstring examples, a new override decorator, and stricter protocol subclass checks.

Changes

File(s)	Change Summary
Lib/test/test_typing.py	Removed `@unittest.expectedFailure` decorators and related TODO comments from multiple test methods.
Lib/typing.py	Added docstring examples; implemented and enabled the `override` decorator; fixed protocol subclass checks; enhanced docstrings with usage examples.
vm/src/protocol/object.rs	Refactored `real_is_instance` and `is_instance` to separate core and recursive isinstance logic, aligning with CPython behavior.

Sequence Diagram(s)

sequenceDiagram
    participant PyObject
    participant VM

    PyObject->>PyObject: is_instance(cls, vm)
    activate PyObject
    PyObject->>PyObject: object_recursive_isinstance(cls, vm)
    alt Exact type match
        PyObject-->>PyObject: Return true
    else Type object
        PyObject->>PyObject: object_isinstance(cls, vm)
        PyObject-->>PyObject: Return result
    else Union type
        loop For each arg in __args__
            PyObject->>PyObject: object_recursive_isinstance(arg, vm)
            alt Any true
                PyObject-->>PyObject: Return true
            end
        end
        PyObject-->>PyObject: Return false
    else Tuple type
        loop For each arg in tuple
            PyObject->>PyObject: object_recursive_isinstance(arg, vm)
            alt Any true
                PyObject-->>PyObject: Return true
            end
        end
        PyObject-->>PyObject: Return false
    else Has __instancecheck__
        PyObject->>VM: Call __instancecheck__
        VM-->>PyObject: Return bool(result)
    else
        PyObject->>PyObject: object_isinstance(cls, vm)
        PyObject-->>PyObject: Return result
    end
    deactivate PyObject

Possibly related PRs

RustPython/RustPython#5844: Also removes @unittest.expectedFailure decorators from typing tests related to protocol subclassing.
RustPython/RustPython#5840: Similar updates to test metadata in Lib/test/test_typing.py with strict rules on modifying test decorators.
RustPython/RustPython#5867: Refactors subclass and class validity checks in vm/src/protocol/object.rs, related to instance and subclass checking logic.

Suggested reviewers

arihant2math

Poem

In the warren of code, a bunny hops,
Refactoring checks, making fewer stops.
Typing grows clearer, with docstrings anew,
Protocols stricter, and tests shining through.
With override marked, and logic aligned,
This patch leaves no bug behind!
🐇✨

✨ Finishing Touches

📝 Generate Docstrings

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)

vm/src/frame.rs (1)
1267-1285: LGTM! Excellent CPython compatibility improvement.

The implementation correctly handles both None and tuple values for type parameters, matching CPython's behavior for PEP 695 type aliasing. The logic is clear and well-commented.

Minor suggestion: Consider updating the error message to be more descriptive:
-            .map_err(|_| vm.new_type_error("Type params must be a tuple."))?
+            .map_err(|_| vm.new_type_error("Type params must be a tuple or None."))?
vm/src/stdlib/typing.rs (1)
558-564: Minor inefficiency & potential borrow bug in repr_str

Calling zelf.__name__() goes through the Python attribute machinery and relies on auto-deref from Py<…> to &ParamSpec.
Directly reading the stored field, as done for TypeVar / TypeVarTuple, avoids that overhead and side-steps any future borrow-checker surprises.
-            let name = zelf.__name__().str(vm)?;
-            Ok(format!("~{}", name))
+            let name = zelf.name.str(vm)?;
+            Ok(format!("~{name}"))
No change in behaviour; only leaner and more idiomatic.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5ccd4f and fd52673.

📒 Files selected for processing (5)

Lib/test/test_typing.py (0 hunks)
Lib/typing.py (8 hunks)
compiler/codegen/src/compile.rs (1 hunks)
vm/src/frame.rs (1 hunks)
vm/src/stdlib/typing.rs (2 hunks)

💤 Files with no reviewable changes (1)

Lib/test/test_typing.py

🧰 Additional context used

📓 Path-based instructions (1)

`Lib/**/*`: Files in the Lib/ directory (Python standard library copied from CPy...

Lib/**/*: Files in the Lib/ directory (Python standard library copied from CPython) should be edited very conservatively; modifications should be minimal and only to work around RustPython limitations.

📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)

List of files the instruction was applied to:

Lib/typing.py

🧬 Code Graph Analysis (1)

vm/src/stdlib/typing.rs (4)

vm/src/builtins/type.rs (4)

repr_str (1301-1320)

zelf (1261-1261)

zelf (1263-1263)

name (430-435)

vm/src/builtins/function.rs (2)

repr_str (570-576)

repr_str (803-817)

vm/src/builtins/genericalias.rs (1)

repr_str (488-490)

vm/src/builtins/descriptor.rs (2)

repr_str (148-154)

repr_str (331-337)

⏰ Context from checks skipped due to timeout of 90000ms (9)

GitHub Check: Check Rust code with rustfmt and clippy
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run snippets and cpython tests on wasm-wasi
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Check the WASM package and demo
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Ensure compilation on various targets

🔇 Additional comments (12)

vm/src/stdlib/typing.rs (1)

346-347: ParamSpec now advertises Representable – good addition

Exposing a custom __repr__ on ParamSpec brings it in-line with TypeVar/TypeVarTuple, avoids falling back to the generic object repr, and unblocks doctest expectations.
Looks correct and self-contained.

compiler/codegen/src/compile.rs (3)

1045-1062: Well-implemented PEP 695 type parameter handling!

The implementation correctly handles the complex compilation sequence for type aliases with type parameters:

Symbol table management: Properly pushes and pops symbol table for type parameter scope

Compilation order: Type parameters compiled first to make them available during value expression compilation

Stack manipulation: Uses Rotate2 to reorder stack from [type_params, value] to [value, type_params] as required by TypeAlias instruction

The extensive comments clearly explain the rationale and stack states, making this complex compilation logic maintainable.

1063-1068: Correct handling of type aliases without parameters.

The else branch properly handles the simpler case where no type parameters are present:

Compiles value expression first

Pushes None (not empty tuple) to represent absence of type parameters, matching CPython behavior

This ensures consistent stack layout for the TypeAlias instruction regardless of whether type parameters are present.

1070-1076: Final stack preparation and instruction emission looks correct.

The code correctly:

Pushes the alias name as the final stack element

Emits the TypeAlias instruction with the properly ordered stack: [value, type_params_or_none, name]

Stores the resulting type alias using the name

This completes the PEP 695 type alias compilation implementation properly.

Lib/typing.py (8)

223-224: LGTM! Good documentation enhancement.

The ParamSpec examples in the docstring clearly demonstrate the intended behavior and align with the function's purpose.

268-269: LGTM! Clear documentation examples.

The examples effectively demonstrate how _collect_type_parameters works with ParamSpec and TypeVar combinations.

2097-2101: LGTM! Proper protocol subclass checking restored.

This change activates previously commented-out code that correctly raises TypeError when protocols with non-method members are used with issubclass(), providing a helpful error message with sorted member names.

534-544: LGTM! Comprehensive documentation examples.

The examples cover a wide range of type constructs and clearly demonstrate get_origin's behavior across different scenarios.

567-570: LGTM! Clear examples for complex type argument extraction.

The examples effectively demonstrate get_args's behavior with Union simplification, TypeVar substitution, and Callable argument extraction.

248-258: LGTM! Practical TypedDict usage examples.

The examples clearly demonstrate TypedDict class definition, type checking behavior, and runtime equality with regular dictionaries.

3715-3751: Consider if this addition aligns with conservative editing guidelines.

The override decorator implementation is well-written and follows established patterns (similar to the final decorator), but this represents a significant new feature addition to the stdlib code. Given the coding guidelines stating that "modifications should be minimal and only to work around RustPython limitations," this change may be more substantial than intended for conservative editing.

However, if this is part of coordinated PEP 698 support implementation, it appears correctly implemented with proper error handling and documentation.

Could you verify if adding the override decorator aligns with the project's conservative editing approach for stdlib files, or if this is part of a broader PEP 698 implementation effort?

777-785: LGTM! Clear protocol members example.

The example effectively demonstrates how get_protocol_members works with a Protocol class and shows the expected frozenset return value.

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (4)

vm/src/builtins/type.rs (2)
158-165: Consider using the any iterator method

The loop can be simplified using iterator methods for better readability.

Apply this diff:
-fn is_subtype_with_mro(a_mro: &[PyTypeRef], _a: &Py<PyType>, b: &Py<PyType>) -> bool {
-    for item in a_mro {
-        if item.is(b) {
-            return true;
-        }
-    }
-    false
+fn is_subtype_with_mro(a_mro: &[PyTypeRef], _a: &Py<PyType>, b: &Py<PyType>) -> bool {
+    a_mro.iter().any(|item| item.is(b))
 }
158-159: Remove or document unused parameter

The _a parameter is not used in the function.

Either remove it if not needed:
-fn is_subtype_with_mro(a_mro: &[PyTypeRef], _a: &Py<PyType>, b: &Py<PyType>) -> bool {
+fn is_subtype_with_mro(a_mro: &[PyTypeRef], b: &Py<PyType>) -> bool {
And update the call site at line 455:
-        is_subtype_with_mro(&*self.mro.read(), self, other)
+        is_subtype_with_mro(&*self.mro.read(), other)
Or add a comment explaining why it's kept for future use.
vm/src/protocol/object.rs (2)
444-445: Remove unnecessary assertion

The assertion assert!(n >= 2) is redundant since the condition is already guaranteed by the match arm that breaks to this point.
-        let n = bases.len();
-        assert!(n >= 2);
-
+        let n = bases.len();
503-507: Potential simplification opportunity

The union handling creates a clone that might be avoidable.

Consider returning a borrowed reference to avoid cloning:
-            if let Ok(union) = cls.try_to_ref::<crate::builtins::PyUnion>(vm) {
-                union.get_args().clone().into()
-            } else {
-                cls.to_owned()
-            }
+            match cls.try_to_ref::<crate::builtins::PyUnion>(vm) {
+                Ok(union) => union.get_args().clone().into(),
+                Err(_) => cls.to_owned()
+            }

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd52673 and 5c082e8.

📒 Files selected for processing (11)

Lib/_py_abc.py (1 hunks)
Lib/abc.py (0 hunks)
Lib/test/test_isinstance.py (7 hunks)
Lib/test/test_typing.py (0 hunks)
Lib/typing.py (8 hunks)
compiler/codegen/src/compile.rs (1 hunks)
vm/src/builtins/type.rs (5 hunks)
vm/src/builtins/union.rs (1 hunks)
vm/src/frame.rs (1 hunks)
vm/src/protocol/object.rs (4 hunks)
vm/src/stdlib/typing.rs (4 hunks)

💤 Files with no reviewable changes (2)

Lib/abc.py
Lib/test/test_typing.py

✅ Files skipped from review due to trivial changes (2)

vm/src/builtins/union.rs
Lib/test/test_isinstance.py

🚧 Files skipped from review as they are similar to previous changes (4)

vm/src/stdlib/typing.rs
compiler/codegen/src/compile.rs
vm/src/frame.rs
Lib/typing.py

🧰 Additional context used

📓 Path-based instructions (1)

`Lib/**/*`: Files in the Lib/ directory (Python standard library copied from CPy...

Lib/**/*: Files in the Lib/ directory (Python standard library copied from CPython) should be edited very conservatively; modifications should be minimal and only to work around RustPython limitations.

📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)

List of files the instruction was applied to:

Lib/_py_abc.py

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When a test fails due to unsupported Python syntax or features, keep the test as '@unittest.expectedFailure' and document the reason, rather than modifying the test logic or data.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Minimize modifications to CPython standard library files in the Lib/ directory; bug fixes should be made through Rust code modifications whenever possible.

vm/src/builtins/type.rs (2)

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Use the macro system (such as 'pyclass', 'pymodule', 'pyfunction') when implementing Python functionality in Rust for RustPython.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When implementing a Python module in Rust for RustPython, use the #[pymodule] attribute and expose functions and classes using #[pyfunction], #[pyclass], and #[pymethod] as appropriate.

🪛 Flake8 (7.2.0)

Lib/_py_abc.py

[error] 114-114: multiple spaces after operator

(E222)

🔇 Additional comments (6)

Lib/_py_abc.py (1)

102-112: Document or remove commented caching code

The caching logic is commented out without explanation. Per the coding guidelines for Lib/ files, modifications should be minimal and well-documented.

Please either:

Remove the commented code if caching is not supported in RustPython

Add a comment explaining why caching is disabled (e.g., # RustPython: Caching temporarily disabled due to...)

vm/src/builtins/type.rs (2)

207-211: LGTM! Proper implementation of PyType_Check

The implementation correctly matches CPython's PyType_Check macro behavior.

1235-1238: Excellent fix for infinite recursion

The changes to __instancecheck__ and __subclasscheck__ correctly avoid infinite recursion by using real_is_instance and real_is_subclass respectively, matching CPython's behavior.

Also applies to: 1241-1245

vm/src/protocol/object.rs (3)

376-387: Well-structured error handling

The check_class method properly validates classes using abstract_get_bases and provides clear error messages.

580-628: Comprehensive instance checking implementation

The enhanced is_instance method correctly handles all cases including exact type checks, unions, tuples, and __instancecheck__ hooks. The implementation matches CPython's behavior closely.

411-531: Excellent refactoring of subclass checking logic

The comprehensive refactoring of abstract_issubclass, recursive_issubclass, and is_subclass methods significantly improves:

Performance with single inheritance optimization

CPython compatibility with proper union and tuple handling

Separation of concerns with real_is_subclass for direct checks

Comment on lines +114 to +115

		r = _abc_instancecheck(cls, instance)
		# print("ABCMeta.__instancecheck__ returning", r)

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix formatting issue and remove debug print

There are two issues here:

Multiple spaces after the = operator
Another debug print statement

Apply this diff:

-        r =  _abc_instancecheck(cls, instance)
-        # print("ABCMeta.__instancecheck__ returning", r)
-        return r
+        return _abc_instancecheck(cls, instance)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	r = _abc_instancecheck(cls, instance)
	# print("ABCMeta.__instancecheck__ returning", r)
	return _abc_instancecheck(cls, instance)

🧰 Tools

🪛 Flake8 (7.2.0)

[error] 114-114: multiple spaces after operator

(E222)

🤖 Prompt for AI Agents

In Lib/_py_abc.py around lines 114 to 115, fix the formatting by removing the
extra spaces after the '=' operator in the assignment to 'r' and delete the
commented-out debug print statement to clean up the code.

Comment on lines +94 to +97

		# import traceback
		# print("ABCMeta.__instancecheck__ called")
		# traceback.print_stack()
		# print("now running _py_abc instancecheck")

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove debug print statements

These debug print statements should not be present in production code.

Apply this diff to remove the debug statements:

-        # import traceback
-        # print("ABCMeta.__instancecheck__ called")
-        # traceback.print_stack()
-        # print("now running _py_abc instancecheck")
-

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	# import traceback
	# print("ABCMeta.__instancecheck__ called")
	# traceback.print_stack()
	# print("now running _py_abc instancecheck")

🤖 Prompt for AI Agents

In Lib/_py_abc.py around lines 94 to 97, remove all commented-out debug print
statements and any related commented imports to clean up the code and ensure no
debug code remains in production.

Comment on lines +427 to 435

		// Avoid recursion in the single inheritance case
		// # safety
		// Intention: bases.as_slice()[0].as_object();
		// Though type-system cannot guarantee, derived does live long enough in the loop.
		derived = unsafe { &(bases.as_slice()[0].as_object() as const _) };
		continue;

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify safety of raw pointer usage

The unsafe block uses a raw pointer to avoid recursion overhead. While the comment explains the intention, this needs careful verification.

The safety comment mentions that derived lives long enough in the loop, but this should be more rigorously verified. Consider adding a more detailed safety justification or using a safer alternative if possible.

🏁 Script executed:

#!/bin/bash
# Search for similar unsafe patterns in the codebase for consistency
rg -A 5 -B 5 "unsafe.*as \*const" --type rust

Length of output: 15325

Add detailed SAFETY justification for raw pointer cast

Please expand the existing // # safety comment in vm/src/protocol/object.rs (around lines 427–432) to spell out exactly why this cast cannot invalidate:

That bases.as_slice()[0] always points to a valid object and bases is never mutated during the loop.
That calling .as_object() returns a reference whose lifetime outlives the assignment to derived.
That casting from *const PyObject to *const ClassType (inferred by _) is safe because they share the same memory layout and alignment.

For example, you might replace the existing comments with:

- // # safety
- // Intention: bases.as_slice()[0].as_object();
- // Though type-system cannot guarantee, derived does live long enough in the loop.
+ // SAFETY:
+ // 1) `bases` is a slice of object references that is not modified inside this loop,
+ //    so `bases[0].as_object()` remains valid until the next iteration or break.
+ // 2) `.as_object()` returns a `&PyObject` whose backing memory is stable for that lifetime.
+ // 3) Casting `*const PyObject` to `*const T` and back preserves layout and does not
+ //    introduce unaligned or dangling references.

No other changes to this pattern are needed, as similar casts elsewhere in the codebase follow the same rationale.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	// Avoid recursion in the single inheritance case
	// # safety
	// Intention: bases.as_slice()[0].as_object();
	// Though type-system cannot guarantee, derived does live long enough in the loop.
	derived = unsafe { &(bases.as_slice()[0].as_object() as const _) };
	continue;
	// Avoid recursion in the single inheritance case
	// SAFETY:
	// 1) `bases` is a slice of object references that is not modified inside this loop,
	// so `bases[0].as_object()` remains valid until the next iteration or break.
	// 2) `.as_object()` returns a `&PyObject` whose backing memory is stable for that lifetime.
	// 3) Casting `const PyObject` to `const T` and back preserves layout and does not
	// introduce unaligned or dangling references.
	derived = unsafe { &(bases.as_slice()[0].as_object() as const _) };
	continue;

🤖 Prompt for AI Agents

In vm/src/protocol/object.rs around lines 427 to 432, expand the existing // #
safety comment to explicitly explain why the raw pointer cast is safe: clarify
that bases.as_slice()[0] always points to a valid object and that bases is not
mutated during the loop, ensuring the reference remains valid; state that
.as_object() returns a reference with a lifetime that outlives the assignment to
derived; and confirm that casting from *const PyObject to *const ClassType is
safe due to identical memory layout and alignment. Replace the current brief
comment with this detailed justification without altering the code logic.

		issubclass() on the whole MRO of a user class, which may contain protocols.
		"""
		return _caller(depth) in {'abc', 'functools', None}
		# XXX: RUSTPYTHON; https://github.com/python/cpython/pull/136115

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🔭 Outside diff range comments (1)

Lib/typing.py (1)
990-991: Include missing CPython PR reference for _py_abc allowlist update

Verified that _py_abc is referenced in:

Lib/typing.py (allowlist)

Lib/abc.py (import fallback)

Lib/test/test_abc.py (tests)

Please update the inline comment in Lib/typing.py (around line 990) to reference the corresponding CPython PR:
- # XXX: RUSTPYTHON; https://github.com/python/cpython/pull/136115
+ # XXX: RUSTPYTHON; aligns with CPython PR 136115 – https://github.com/python/cpython/pull/136115

🧹 Nitpick comments (1)

Lib/typing.py (1)

717-752: Consider using contextlib.suppress for exception handling.

The override decorator implementation is correct, but the exception handling could be more concise.

Apply this change to follow the static analysis suggestion:

+import contextlib
+
 def override[F: _Func](method: F, /) -> F:
     """Indicate that a method is intended to override a method in a base class.
 
     Usage::
 
         class Base:
             def method(self) -> None:
                 pass
 
         class Child(Base):
             @override
             def method(self) -> None:
                 super().method()
 
     When this decorator is applied to a method, the type checker will
     validate that it overrides a method or attribute with the same name on a
     base class.  This helps prevent bugs that may occur when a base class is
     changed without an equivalent change to a child class.
 
     There is no runtime checking of this property. The decorator attempts to
     set the ``__override__`` attribute to ``True`` on the decorated object to
     allow runtime introspection.
 
     See PEP 698 for details.
     """
-    try:
-        method.__override__ = True
-    except (AttributeError, TypeError):
-        # Skip the attribute silently if it is not writable.
-        # AttributeError happens if the object has __slots__ or a
-        # read-only property, TypeError if it's a builtin class.
-        pass
+    with contextlib.suppress(AttributeError, TypeError):
+        method.__override__ = True
     return method

📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2243804 and 3f5566d.

⛔ Files ignored due to path filters (2)

vm/src/protocol/object.rs is excluded by none and included by none
vm/src/stdlib/typing.rs is excluded by none and included by none

📒 Files selected for processing (2)

Lib/test/test_typing.py (0 hunks)
Lib/typing.py (9 hunks)

💤 Files with no reviewable changes (1)

Lib/test/test_typing.py

🧰 Additional context used

📓 Path-based instructions (2)

`**/*.py`: Follow PEP 8 style for custom Python code Use ruff for linting Python code

**/*.py: Follow PEP 8 style for custom Python code
Use ruff for linting Python code

📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)

List of files the instruction was applied to:

Lib/typing.py

`Lib/**/*.py`: Minimize modifications to CPython standard library files in the `Lib/` directory

Lib/**/*.py: Minimize modifications to CPython standard library files in the Lib/ directory

📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)

List of files the instruction was applied to:

Lib/typing.py

🪛 Ruff (0.11.9)

Lib/typing.py

3745-3751: Use contextlib.suppress(AttributeError, TypeError) instead of try-except-pass

(SIM105)

⏰ Context from checks skipped due to timeout of 90000ms (11)

GitHub Check: Run snippets and cpython tests on wasm-wasi
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Ensure compilation on various targets
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Check the WASM package and demo
GitHub Check: Run tests under miri
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Check Rust code with rustfmt and clippy
GitHub Check: Run snippets and cpython tests (macos-latest)

🔇 Additional comments (7)

Lib/typing.py (7)

223-224: LGTM! Clear documentation enhancement.

The doctest examples effectively demonstrate the behavior of _should_unflatten_callable_args with ParamSpec, showing that when using ParamSpec with Callable, the arguments are not flattened.

268-269: LGTM! Helpful doctest examples added.

The examples clearly illustrate how _collect_type_parameters handles both TypeVar and ParamSpec in the correct order of first appearance.

2098-2102: LGTM! Critical protocol subclass check activated.

The uncommented code properly implements the PEP 544 requirement that protocols with non-method members should not support issubclass(). The error message is clear and includes the sorted list of non-method members.

535-545: LGTM! Comprehensive examples enhance documentation.

The examples cover various typing constructs including Literal, ClassVar, Generic, Union, List, and ParamSpec.args, providing clear illustrations of get_origin behavior.

568-571: LGTM! Well-structured examples for get_args.

The examples demonstrate get_args behavior with different type constructs, including the important edge case showing how Union simplification works.

249-259: LGTM! Excellent TypedDict usage examples.

The examples clearly demonstrate class-based TypedDict definition, type checking behavior, and equality with regular dicts, making the documentation much more helpful.

776-786: LGTM! Clear example enhances documentation.

The example effectively demonstrates how to use get_protocol_members to retrieve protocol attributes from a Protocol class.

Upgrade typing to 3.13.5 by youknowone · Pull Request #5850 · RustPython/RustPython

youknowone commented Jun 27, 2025 • edited by coderabbitai Bot Loading

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Possibly related PRs

Suggested reviewers

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowone commented
Jun 27, 2025
•

edited by coderabbitai Bot

Loading