gemini-code-assist[bot] commented on code in PR #621:
URL: https://github.com/apache/tvm-ffi/pull/621#discussion_r3409416413


##########
python/tvm_ffi/cython/type_info.pxi:
##########
@@ -517,12 +532,40 @@ class TypeSchema:
             assert s.repr() == "Array[int]"
 
         """
-        if ty_map is None:
-            origin = self.origin
-        else:
-            origin = ty_map(self.origin)
+        return self.output_repr(ty_map)
+
+    def input_repr(self, ty_map: "Optional[Callable[[str], str]]" = None) -> 
str:
+        """Render the Python input annotation accepted by this schema."""
+        return self._repr_impl(ty_map, input_mode=True, 
expanded_convert_types=frozenset())
+
+    def output_repr(self, ty_map: "Optional[Callable[[str], str]]" = None) -> 
str:
+        """Render the precise Python output annotation produced by this 
schema."""
+        return self._repr_impl(ty_map, input_mode=False, 
expanded_convert_types=frozenset())
+
+    def _repr_impl(
+        self,
+        ty_map: "Optional[Callable[[str], str]]",
+        input_mode: bool,
+        expanded_convert_types: frozenset[int],

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   Using `frozenset[int]` as a type hint directly will raise `TypeError: 'type' 
object is not subscriptable` at runtime on Python 3.8, which this library 
supports. To prevent this, wrap the type hint in a string literal as done 
elsewhere in this file (e.g., `"frozenset[int]"`).
   
   ```
           expanded_convert_types: "frozenset[int]",
   ```



##########
python/tvm_ffi/stub/codegen.py:
##########
@@ -46,6 +46,15 @@ def _run(name: str) -> str:
     return _run
 
 
+def _make_input_ty_map(ty_map: dict[str, str]) -> dict[str, str]:
+    """Derive input-side defaults without overriding explicit ty-map 
entries."""
+    input_ty_map = ty_map.copy()
+    for key, input_default in C.TY_MAP_INPUT_DEFAULTS.items():
+        if ty_map.get(key) == C.TY_MAP_DEFAULTS.get(key):

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   If `ty_map` is a partially populated dictionary (e.g., containing only 
user-defined overrides), `ty_map.get(key)` will return `None` for missing keys. 
Since `C.TY_MAP_DEFAULTS.get(key)` is a string, the comparison `None == 
C.TY_MAP_DEFAULTS.get(key)` will evaluate to `False`, preventing the input-side 
defaults from being correctly populated. Using `ty_map.get(key, 
C.TY_MAP_DEFAULTS.get(key))` provides a robust fallback to the default mapping 
when the key is absent from `ty_map`.
   
   ```suggestion
           if ty_map.get(key, C.TY_MAP_DEFAULTS.get(key)) == 
C.TY_MAP_DEFAULTS.get(key):
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to