tqchen commented on code in PR #20016:
URL: https://github.com/apache/tvm/pull/20016#discussion_r3595689186


##########
python/tvm/ir/expr.py:
##########
@@ -413,11 +413,6 @@ def __init__(
                 raise TypeError("ty must be a Type or primitive dtype string")
         self.__init_handle_by_constructor__(_ffi_api.Var, name, ty, span)
 

Review Comment:
   Thanks for flagging this. I cross-checked the compatibility intent: this 
cleanup intentionally removes Python attribute access through name_hint. The 
narrow name_hint= constructor input and legacy JSON upgrade remain for 
ingestion compatibility, but name is the sole reflected and Python Var 
attribute, so we will not add a deprecated forwarding property.



##########
tests/python/relax/test_expr.py:
##########
@@ -60,8 +60,12 @@ def test_var() -> None:
 
 
 def test_var_name_keyword_compatibility() -> None:
-    assert tvm.ir.Var(name="primary").name == "primary"
-    assert tvm.ir.Var(name_hint="legacy").name == "legacy"
+    primary = tvm.ir.Var(name="primary")
+    legacy = tvm.ir.Var(name_hint="legacy")
+    assert primary.name == "primary"
+    assert legacy.name == "legacy"
+    assert not hasattr(primary, "name_hint")
+    assert not hasattr(legacy, "name_hint")

Review Comment:
   The absence assertions are intentional after the compatibility cross-check. 
They codify that Python attribute access is cleaned up to name only, while this 
same test still covers the accepted legacy name_hint= constructor input.



##########
tests/python/relax/test_expr.py:
##########
@@ -102,8 +106,12 @@ def test_dataflow_var() -> None:
 
 
 def test_dataflow_var_name_keyword_compatibility() -> None:
-    assert rx.DataflowVar(name="primary").name == "primary"
-    assert rx.DataflowVar(name_hint="legacy").name == "legacy"
+    primary = rx.DataflowVar(name="primary")
+    legacy = rx.DataflowVar(name_hint="legacy")
+    assert primary.name == "primary"
+    assert legacy.name == "legacy"
+    assert not hasattr(primary, "name_hint")
+    assert not hasattr(legacy, "name_hint")

Review Comment:
   Likewise for DataflowVar, the absence assertion is intentional: it inherits 
the cleaned-up Var attribute API, while its legacy name_hint= constructor input 
remains covered separately.



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