-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Upgrade typing to 3.13.5 #5850
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.
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
Upgrade typing to 3.13.5 #5850
Conversation
WalkthroughThis change refactors the Changes
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
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
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 inrepr_str
Calling
zelf.__name__()
goes through the Python attribute machinery and relies on auto-deref fromPy<…>
to&ParamSpec
.
Directly reading the stored field, as done forTypeVar
/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
📒 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 advertisesRepresentable
– good additionExposing a custom
__repr__
onParamSpec
brings it in-line withTypeVar
/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 byTypeAlias
instructionThe 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 behaviorThis 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 thefinal
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.
There was a problem hiding this comment.
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 theany
iterator methodThe 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 parameterThe
_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 assertionThe 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 opportunityThe 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
📒 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 codeThe 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_CheckThe implementation correctly matches CPython's PyType_Check macro behavior.
1235-1238
: Excellent fix for infinite recursionThe changes to
__instancecheck__
and__subclasscheck__
correctly avoid infinite recursion by usingreal_is_instance
andreal_is_subclass
respectively, matching CPython's behavior.Also applies to: 1241-1245
vm/src/protocol/object.rs (3)
376-387
: Well-structured error handlingThe
check_class
method properly validates classes usingabstract_get_bases
and provides clear error messages.
580-628
: Comprehensive instance checking implementationThe 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 logicThe comprehensive refactoring of
abstract_issubclass
,recursive_issubclass
, andis_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
Lib/_py_abc.py
Outdated
r = _abc_instancecheck(cls, instance) | ||
# print("ABCMeta.__instancecheck__ returning", r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Lib/_py_abc.py
Outdated
# import traceback | ||
# print("ABCMeta.__instancecheck__ called") | ||
# traceback.print_stack() | ||
# print("now running _py_abc instancecheck") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
vm/src/protocol/object.rs
Outdated
// 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; |
There was a problem hiding this comment.
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 andbases
is never mutated during the loop. - That calling
.as_object()
returns a reference whose lifetime outlives the assignment toderived
. - 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.
@@ -1983,7 +1987,8 @@ def _allow_reckless_class_checks(depth=2): | |||
The abc and functools modules indiscriminately call isinstance() and | |||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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 updateVerified 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
⛔ Files ignored due to path filters (2)
vm/src/protocol/object.rs
is excluded by none and included by nonevm/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 theLib/
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
withParamSpec
, showing that when usingParamSpec
withCallable
, the arguments are not flattened.
268-269
: LGTM! Helpful doctest examples added.The examples clearly illustrate how
_collect_type_parameters
handles bothTypeVar
andParamSpec
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
, andParamSpec.args
, providing clear illustrations ofget_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 howUnion
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.
Summary by CodeRabbit
New Features
override
decorator to mark methods as overrides at runtime.issubclass()
.Documentation
Refactor
Tests