[PYTHON] Fix PEP 563 compat and remove args_converter#18847
[PYTHON] Fix PEP 563 compat and remove args_converter#18847tqchen merged 3 commits intoapache:mainfrom
Conversation
This PR resolves string annotations from PEP 563 (from __future__ import annotations) at decoration time using typing.get_type_hints(), preventing TypeError crashes when @type_checked passes stringified annotations to isinstance().
This PR removes the @args_converter.auto decorator that auto-converted Python types to Relax Expr by inspecting annotations at decoration time. This was PEP 563-incompatible (annotations become strings). The FFI/C++ layer handles type conversion directly, with minimal explicit conversion added at call sites for Python tuples/lists and primitive types.
Summary of ChangesHello, 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 refactors the argument conversion strategy within the Relax framework. It addresses compatibility issues with Python's PEP 563 for type annotations and transitions from an implicit auto-conversion decorator to explicit conversion calls. This change improves the clarity and maintainability of the code by making argument type handling more direct, while also streamlining the type-checking process. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
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
|
There was a problem hiding this comment.
Code Review
This pull request is a nice refactoring that removes the implicit args_converter decorator in favor of explicit conversions, which improves code clarity. It also includes an important fix for PEP 563 compatibility in the type checker. My review focuses on ensuring the new explicit conversions are correct and complete across all modified files. I've identified several instances where the conversion logic was either incomplete (e.g., not handling PrimExpr) or missing, which could lead to runtime errors. I've provided suggestions to fix these issues. Additionally, I've included some recommendations for code simplification and improving robustness.
- Fix distributed ops (distributed.py, distributed/ir.py): apply convert_to_expr element-wise when args is a list/tuple before constructing RxTuple, so non-Expr elements are properly converted. - Simplify redundant isinstance(x, Expr) guards before convert_to_expr calls (convert_to_expr is idempotent on Expr inputs) in: memory/memory.py, op/base.py, op/index.py, op/unary.py, op/vm/vm.py, script/ir_builder/relax/ir.py.
7adf2c9 to
8aba2c4
Compare
Summary
Fix type_checked for PEP 563 compatibility and remove args_converter.auto decorator.
Changes