style(fuzz): Modernize type annotations to Python 3.10+ syntax#251
style(fuzz): Modernize type annotations to Python 3.10+ syntax#251Hzfengsy merged 1 commit intohw-native-sys:mainfrom
Conversation
Summary of ChangesHello @lwDavid, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the maintainability and readability of the fuzz testing framework by adopting modern Python type hinting conventions. The changes primarily involve updating type annotations to align with Python 3.10+ syntax, which simplifies the code and improves type checking. Additionally, minor documentation formatting adjustments were made to ensure consistency and clarity. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThe pull request modernizes Python type hints across the fuzzing test framework from typing module imports (List, Dict, Optional, Tuple) to Python 3.9+ built-in generics (list, dict, tuple, | union syntax). Documentation is enhanced with new configuration parameters and component descriptions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request effectively modernizes the type annotations in the fuzz testing framework to the Python 3.10+ syntax, replacing legacy typing imports with built-in generics and the | union operator. The changes are consistent and correctly applied across all fuzz test source files, improving code readability and adhering to modern Python standards. Additionally, the formatting fixes in README.md enhance the documentation's clarity. The implementation is solid, and I have no further suggestions for improvement.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/st/fuzz/src/fuzzer.py (1)
1077-1085: Markdefault_shapeas intentionally unused.
default_shapeisn’t referenced in_try_expand_row_vecand_try_expand_col_vec(Line 1084, Line 1166), which triggers ARG002. Consider explicitly marking it unused.♻️ Suggested lint-silencing tweak
@@ def _try_expand_row_vec( self, row_vec_name: str, operations: list[dict[str, Any]], available_tiles: list[str], variable_shapes: dict[str, tuple[int, int]], variable_usage_count: dict[str, int], default_shape: tuple[int, int], next_tmp_index: int, ) -> int: + _ = default_shape # reserved for future fallback logic # [M, N] tile (N != 1) @@ def _try_expand_col_vec( self, col_vec_name: str, operations: list[dict[str, Any]], available_tiles: list[str], variable_shapes: dict[str, tuple[int, int]], variable_usage_count: dict[str, int], variable_ranges: dict[str, "ValueRange"], default_shape: tuple[int, int], next_tmp_index: int, ) -> int: + _ = default_shape # reserved for future fallback logic # Get [1, N] shapeAlso applies to: 1158-1167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/st/fuzz/src/fuzzer.py` around lines 1077 - 1085, The parameter default_shape in the functions _try_expand_row_vec and _try_expand_col_vec is intentionally unused and triggers ARG002; rename the parameter to _default_shape (or prefix it with an underscore) in both function signatures and any callers to signal it is intentionally unused and silence the lint warning, ensuring you update the identifiers for _try_expand_row_vec and _try_expand_col_vec consistently.tests/st/fuzz/src/multi_kernel_test_generator.py (1)
307-309: Silence unused-parameter warnings fororch_info/shape.
orch_infoandshapeare not referenced (Line 308, Line 656–657), so Ruff raises ARG002. Consider marking them intentionally unused.♻️ Suggested lint-silencing tweak
@@ def _generate_torch_reference( self, kernels: list[dict[str, Any]], orch_info: dict[str, Any], ) -> str: + _ = orch_info # reserved for future use """Torch reference implementation @@ def _validate_golden_output( # noqa: PLR0912 self, kernels: list[dict[str, Any]], orch_info: dict[str, Any], shape: tuple[int, int], tensor_init_type: str, ) -> None: + _ = orch_info # reserved for future use + _ = shape # reserved for future use """golden , NaN/InfAlso applies to: 655-658
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/st/fuzz/src/multi_kernel_test_generator.py` around lines 307 - 309, The parameters orch_info and shape in multi_kernel_test_generator.py are unused and trigger ARG002; silence this by marking them intentionally unused: either rename them to _orch_info and _shape in the function signatures that declare them, or add a single-line no-op reference inside the function body (e.g. _ = orch_info; _ = shape) or append a per-parameter noqa comment (e.g. # noqa: ARG002) — do this for every function that declares orch_info and shape so Ruff stops flagging them.tests/st/fuzz/src/orchestrator_generator.py (1)
35-39: Silence Ruff ARG002 for the unusedshapeparameter.
shapeisn’t referenced in these methods (Line 38, Line 112), which triggers lint noise. Consider explicitly marking it unused to keep the public signature intact.♻️ Suggested lint-silencing tweak
@@ def generate_sequential( self, kernels: list[dict[str, Any]], shape: tuple[int, int] = (128, 128), ) -> dict[str, Any]: + _ = shape # reserved for future shape-specific logic if not kernels: raise ValueError("kernels") @@ def generate_branching( self, kernels: list[dict[str, Any]], shape: tuple[int, int] = (128, 128), ) -> dict[str, Any]: + _ = shape # reserved for future shape-specific logic if not kernels: raise ValueError("kernels")Also applies to: 109-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/st/fuzz/src/orchestrator_generator.py` around lines 35 - 39, The shape parameter is unused but must remain in the public signature; to silence Ruff ARG002, consume it inside the function body (without changing the parameter name) by assigning it to a throwaway variable—e.g. add a line like _ = shape # silence ruff ARG002—inside generate_sequential and the other generator method that also accepts shape (the one around lines 109–113) so the parameter is considered used while the signature stays intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/st/fuzz/src/fuzzer.py`:
- Around line 1077-1085: The parameter default_shape in the functions
_try_expand_row_vec and _try_expand_col_vec is intentionally unused and triggers
ARG002; rename the parameter to _default_shape (or prefix it with an underscore)
in both function signatures and any callers to signal it is intentionally unused
and silence the lint warning, ensuring you update the identifiers for
_try_expand_row_vec and _try_expand_col_vec consistently.
In `@tests/st/fuzz/src/multi_kernel_test_generator.py`:
- Around line 307-309: The parameters orch_info and shape in
multi_kernel_test_generator.py are unused and trigger ARG002; silence this by
marking them intentionally unused: either rename them to _orch_info and _shape
in the function signatures that declare them, or add a single-line no-op
reference inside the function body (e.g. _ = orch_info; _ = shape) or append a
per-parameter noqa comment (e.g. # noqa: ARG002) — do this for every function
that declares orch_info and shape so Ruff stops flagging them.
In `@tests/st/fuzz/src/orchestrator_generator.py`:
- Around line 35-39: The shape parameter is unused but must remain in the public
signature; to silence Ruff ARG002, consume it inside the function body (without
changing the parameter name) by assigning it to a throwaway variable—e.g. add a
line like _ = shape # silence ruff ARG002—inside generate_sequential and the
other generator method that also accepts shape (the one around lines 109–113) so
the parameter is considered used while the signature stays intact.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tests/st/fuzz/README.mdtests/st/fuzz/src/fuzzer.pytests/st/fuzz/src/kernel_generator.pytests/st/fuzz/src/multi_kernel_test_generator.pytests/st/fuzz/src/orchestrator_generator.py
|
@Hzfengsy Request review. |
style(fuzz): modernize type annotations to Python 3.10+ syntax
Replace legacy typing imports (Dict, List, Tuple, Optional) with
built-in generics (dict, list, tuple) and union syntax (X | None)
across all fuzz test source files. Also fix minor README formatting
(code block language tag, blank lines before lists, table alignment).