GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Update dataclasses, inspect from CPython 3.13 by youknowone · Pull Request #5377 · RustPython/RustPython

Conversation

youknowone commented Jul 29, 2024 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Summary by CodeRabbit

Refactor
- Improved singleton value conversion and error reporting for invalid inputs.
- Expanded and unified handling of class-pattern components so pattern arguments and keyword attributes/patterns fully round-trip to/from AST representations.
- Standardized serialization/deserialization signatures across related AST structures for more consistent behavior.

_{✏️ 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.

All reactions

youknowone force-pushed the inspect branch from 834154d to 29a8712 Compare

July 29, 2024 11:42

youknowone changed the title ~~Update inspect from CPython 3.12~~ Update dataclasses, inspect from CPython 3.12

Jul 29, 2024

youknowone mentioned this pull request

Jul 29, 2024

Update Python libraries and tests from CPython 3.12 #5104

Closed

youknowone force-pushed the inspect branch 3 times, most recently from faed11a to a3579b3 Compare

July 29, 2024 23:47

youknowone force-pushed the inspect branch 3 times, most recently from 4937811 to b1235fe Compare

March 30, 2025 05:51

youknowone force-pushed the inspect branch from b1235fe to 319d225 Compare

December 12, 2025 10:47

coderabbitai Bot commented Dec 12, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Walkthrough

Extended AST pattern conversions: added full bidirectional mapping for ruff::Singleton (None/True/False) and converted several PatternMatchClass-related types into vector wrappers, updated split/merge utilities and Node impls to use the new wrappers and revised function signatures.

Changes

Cohort / File(s)	Summary
Singleton Conversion `crates/vm/src/stdlib/ast/pattern.rs`	Implemented `ast_to_object` mapping for `ruff::Singleton` → VM objects (`none`, `true`, `false`) and `ast_from_object` recognizing VM objects and returning `Singleton` variants; adjusted parameter names/signatures.
PatternMatchClass wrapper types `crates/vm/src/stdlib/ast/pattern.rs`	Replaced three placeholder structs with wrappers: `PatternMatchClassPatterns(Vec<ruff::Pattern>)`, `PatternMatchClassKeywordAttributes(Vec<ruff::Identifier>)`, `PatternMatchClassKeywordPatterns(Vec<ruff::Pattern>)`. Each now delegates `ast_to_object`/`ast_from_object` to the inner vectors.
split/merge utilities & Node routing `crates/vm/src/stdlib/ast/pattern.rs`	`split_pattern_match_class` now returns the new wrapper types built from `ruff::PatternArguments`. `merge_pattern_match_class` accepts the wrappers and returns `(Vec<ruff::Pattern>, Vec<ruff::PatternKeyword>)` by zipping attributes and patterns. Node implementations updated to route through new wrappers; several function signatures adjusted to `(vm: &VirtualMachine, source_file: &SourceFile, object: PyObjectRef)` style.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Pay attention to correctness of conversions between wrappers and inner vectors (edge cases: empty lists, mismatched lengths).
Validate merge_pattern_match_class zipping logic produces correct PatternKeyword ordering and error handling.
Confirm all ast_to_object / ast_from_object signature changes match trait expectations and are used consistently across Node impls.
Review tests or add ones for round-tripping class-pattern structures and singleton conversions.

Poem

🐇 I nibbled bytes beneath moonlight’s beam,
Turned None to none and True to gleam.
Patterns gathered, wrapped in a row,
Split, merged, hopped — now round-trips flow.
Code carrots crunch; the AST sings “go!” 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title 'Update dataclasses, inspect from CPython 3.13' does not match the actual changes shown in the summary, which focus entirely on AST pattern matching class handling in the Rust VM, not dataclasses or inspect module updates.	Revise the title to accurately reflect the primary change, such as 'Implement AST pattern-match class conversion for None/True/False singletons' or 'Add roundtrip AST conversion for class pattern components'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone changed the title ~~Update dataclasses, inspect from CPython 3.12~~ Update dataclasses, inspect from CPython 3.13

Dec 12, 2025

youknowone force-pushed the inspect branch 2 times, most recently from d4d8bda to b6dfa84 Compare

December 12, 2025 12:01

coderabbitai Bot reviewed

Dec 12, 2025

View reviewed changes

coderabbitai Bot left a comment

Copy link

Copy Markdown

Contributor

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 (1)

crates/vm/src/stdlib/ast/pattern.rs (1)
210-227: Correct object-to-singleton conversion with minor formatting nit.

The identity checks and error handling are appropriate. The conversion logic correctly distinguishes between the three singleton values.

Consider using display format {} instead of debug format {:?} in the error message for cleaner output:
         } else {
             Err(vm.new_value_error(format!(
-                "Expected None, True, or False, got {:?}",
+                "Expected None, True, or False, got {}",
                 object.class().name()
             )))
         }

📜 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 75dcf80 and b6dfa84.

⛔ Files ignored due to path filters (26)

Lib/dataclasses.py is excluded by !Lib/**
Lib/dis.py is excluded by !Lib/**
Lib/inspect.py is excluded by !Lib/**
Lib/test/__init__.py is excluded by !Lib/**
Lib/test/test__opcode.py is excluded by !Lib/**
Lib/test/test_abc.py is excluded by !Lib/**
Lib/test/test_compile.py is excluded by !Lib/**
Lib/test/test_copy.py is excluded by !Lib/**
Lib/test/test_dataclasses/__init__.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_module_1.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_module_1_str.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_module_2.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_module_2_str.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_textanno.py is excluded by !Lib/**
Lib/test/test_enum.py is excluded by !Lib/**
Lib/test/test_fstring.py is excluded by !Lib/**
Lib/test/test_inspect/__init__.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_fodder.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_fodder2.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_stock_annotations.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_stringized_annotations.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_stringized_annotations_2.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_stringized_annotations_pep695.py is excluded by !Lib/**
Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**
Lib/test/test_warnings/__init__.py is excluded by !Lib/**

📒 Files selected for processing (1)

crates/vm/src/stdlib/ast/pattern.rs (1 hunks)

🧰 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 Rust 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/stdlib/ast/pattern.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

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

🔇 Additional comments (1)

crates/vm/src/stdlib/ast/pattern.rs (1)

202-208: LGTM! Clean singleton-to-object conversion.

The exhaustive pattern match correctly maps each Singleton variant to its corresponding VM object representation.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

youknowone force-pushed the inspect branch from b6dfa84 to 1b901a1 Compare

December 12, 2025 12:12

youknowone added 6 commits

December 12, 2025 21:43


          impl more ast

be5f660


          inspect from v3.13.10

d22956e


          Upgrade dataclasses from CPython 3.13.10

d116049


          hybrid dis.py from CPython 3.13.10

152d10b


          patch test_inspect

ef6c6ad


          mark expectedFailure

9bbfdf5

youknowone force-pushed the inspect branch from 1b901a1 to 9bbfdf5 Compare

December 12, 2025 12:44

coderabbitai Bot reviewed

Dec 12, 2025

View reviewed changes

coderabbitai Bot left a comment

Copy link

Copy Markdown

Contributor

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: 1

🧹 Nitpick comments (1)

crates/vm/src/stdlib/ast/pattern.rs (1)

543-558: Avoid cloning kwd_attrs (optional)

split_pattern_match_class currently clones attr because it later consumes arguments.keywords. You can split attrs/patterns in a single consuming loop to avoid clones.

📜 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 b6dfa84 and 9bbfdf5.

⛔ Files ignored due to path filters (25)

Lib/dataclasses.py is excluded by !Lib/**
Lib/dis.py is excluded by !Lib/**
Lib/inspect.py is excluded by !Lib/**
Lib/test/test__opcode.py is excluded by !Lib/**
Lib/test/test_abc.py is excluded by !Lib/**
Lib/test/test_compile.py is excluded by !Lib/**
Lib/test/test_copy.py is excluded by !Lib/**
Lib/test/test_dataclasses/__init__.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_module_1.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_module_1_str.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_module_2.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_module_2_str.py is excluded by !Lib/**
Lib/test/test_dataclasses/dataclass_textanno.py is excluded by !Lib/**
Lib/test/test_enum.py is excluded by !Lib/**
Lib/test/test_fstring.py is excluded by !Lib/**
Lib/test/test_inspect/__init__.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_fodder.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_fodder2.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_stock_annotations.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_stringized_annotations.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_stringized_annotations_2.py is excluded by !Lib/**
Lib/test/test_inspect/inspect_stringized_annotations_pep695.py is excluded by !Lib/**
Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
Lib/test/test_unittest/testmock/testhelpers.py is excluded by !Lib/**
Lib/test/test_warnings/__init__.py is excluded by !Lib/**

📒 Files selected for processing (1)

crates/vm/src/stdlib/ast/pattern.rs (3 hunks)

🧰 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 Rust 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/stdlib/ast/pattern.rs

🧬 Code graph analysis (1)

crates/vm/src/stdlib/ast/pattern.rs (5)

crates/vm/src/stdlib/itertools.rs (1)

None (1152-1152)

crates/vm/src/stdlib/ast/other.rs (1)

object (39-39)

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

arguments (66-66)

crates/vm/src/frame.rs (1)

kwd_attrs (1122-1122)

crates/vm/src/stdlib/functools.rs (1)

keywords (72-74)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

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

🔇 Additional comments (1)

crates/vm/src/stdlib/ast/pattern.rs (1)

390-435: Wrapper types look fine; delegation is clean

The tuple-struct wrappers + Node delegation keep the PatternMatchClass conversion code simpler and should round-trip as long as Vec<T>: Node and ruff::Identifier: Node behave as expected.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/vm/src/stdlib/ast/pattern.rs

Comment on lines 560 to 578

		/// Merges the pattern match class attributes and patterns, opposite of [`split_pattern_match_class`].
		fn merge_pattern_match_class(
		_patterns: PatternMatchClassPatterns,
		_kwd_attrs: PatternMatchClassKeywordAttributes,
		_kwd_patterns: PatternMatchClassKeywordPatterns,
		patterns: PatternMatchClassPatterns,
		kwd_attrs: PatternMatchClassKeywordAttributes,
		kwd_patterns: PatternMatchClassKeywordPatterns,
		) -> (Vec<ruff::Pattern>, Vec<ruff::PatternKeyword>) {
		todo!()
		let keywords = kwd_attrs
		.0
		.into_iter()
		.zip(kwd_patterns.0)
		.map(\|(attr, pattern)\| ruff::PatternKeyword {
		range: Default::default(),
		node_index: Default::default(),
		attr,
		pattern,
		})
		.collect();
		(patterns.0, keywords)
		}

coderabbitai Bot Dec 12, 2025

Copy link

Copy Markdown

Contributor

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.

⚠️ Potential issue | 🟠 Major

Don’t silently truncate mismatched kwd_attrs / kwd_patterns lengths

zip will drop extras if lengths differ. Since ast_from_object accepts user-constructed AST objects, this should error instead of losing data.

Low-impact fix: validate in ruff::PatternMatchClass::ast_from_object (where vm is available) before calling merge_pattern_match_class:

 let kwd_attrs = Node::ast_from_object(
     vm, source_file, get_node_field(vm, &object, "kwd_attrs", "MatchClass")?
 )?;
 let kwd_patterns = Node::ast_from_object(
     vm, source_file, get_node_field(vm, &object, "kwd_patterns", "MatchClass")?
 )?;
+
+if kwd_attrs.0.len() != kwd_patterns.0.len() {
+    return Err(vm.new_value_error(format!(
+        "MatchClass kwd_attrs and kwd_patterns must have the same length ({} != {})",
+        kwd_attrs.0.len(),
+        kwd_patterns.0.len(),
+    )));
+}
 let (patterns, keywords) = merge_pattern_match_class(patterns, kwd_attrs, kwd_patterns);

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In crates/vm/src/stdlib/ast/pattern.rs around lines 560 to 578, the current
merge_pattern_match_class uses zip and silently drops extra entries when
kwd_attrs and kwd_patterns lengths differ; instead, validate that
kwd_attrs.0.len() == kwd_patterns.0.len() before merging (preferably in
ruff::PatternMatchClass::ast_from_object where the VM is available) and
return/raise a TypeError (or appropriate VM error) if they differ with a clear
message indicating mismatched keyword attributes and patterns lengths; only
after validation, construct PatternKeyword entries and return the merged
vectors.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

youknowone merged commit 093ba25 into RustPython:main

Dec 12, 2025

13 checks passed

Uh oh!

There was an error while loading. Please reload this page.

youknowone deleted the inspect branch

December 12, 2025 13:44

youknowone mentioned this pull request

Dec 12, 2025

Update test/libregrtest to CPython 3.13.10 #6410

Merged

2 tasks done

coderabbitai Bot mentioned this pull request

Feb 6, 2026

Refactor _ast module for clarity #7031

Merged

YangSiJun528 mentioned this pull request

Oct 6, 2026

Review issues that appear resolved for closure #8982

Closed

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

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.

1 participant

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

youknowone commented Jul 29, 2024 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Jul 29, 2024 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Dec 12, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.