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


##########
tests/python/test_tensor.py:
##########
@@ -131,5 +131,14 @@ def _check_device(x: tvm_ffi.Tensor) -> str:
     assert device_type == "rocm"
 
 
+def test_optional_tensor_view() -> None:
+    x = np.zeros((128,), dtype="float32")
+    optional_tensor_view_has_value = tvm_ffi.get_global_func(
+        "testing.optional_tensor_view_has_value"
+    )
+    assert not optional_tensor_view_has_value(None)
+    assert optional_tensor_view_has_value(x)

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The test is correct, but it can be improved by using 
`pytest.mark.parametrize`. This makes the test more idiomatic to pytest, more 
readable by separating test data from logic, and easier to extend with more 
test cases in the future. This refactoring also adds a check for DLPack support 
in NumPy, skipping the test case if not available, which improves robustness.
   
   ```suggestion
   @pytest.mark.parametrize(
       "arg, expected",
       [
           (None, False),
           (np.zeros((128,), dtype="float32"), True),
       ],
   )
   def test_optional_tensor_view(arg, expected) -> None:
       optional_tensor_view_has_value = tvm_ffi.get_global_func(
           "testing.optional_tensor_view_has_value"
       )
   
       if arg is not None and not hasattr(arg, "__dlpack__"):
           pytest.skip("NumPy version does not support DLPack.")
   
       assert optional_tensor_view_has_value(arg) is expected
   ```



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