gemini-code-assist[bot] commented on code in PR #379:
URL: https://github.com/apache/tvm-ffi/pull/379#discussion_r2657452314
##########
python/tvm_ffi/container.py:
##########
@@ -375,3 +387,15 @@ def __repr__(self) -> str:
if self.__chandle__() == 0:
return type(self).__name__ + "(chandle=None)"
return "{" + ", ".join([f"{k.__repr__()}: {v.__repr__()}" for k, v in
self.items()]) + "}"
+
+ def __eq__(self, other: object) -> bool:
+ """Return True if the map is equal to another map."""
+ return _ffi_api.MapEqual(self, other)
+
+ def __ne__(self, other: object) -> bool:
+ """Return True if the map is not equal to another map."""
+ return not _ffi_api.MapEqual(self, other)
Review Comment:
Similar to the `Array` class, the `__ne__` method for `Map` can be removed.
Python will automatically provide the correct `__ne__` behavior based on the
`__eq__` implementation, making this explicit definition redundant.
##########
python/tvm_ffi/container.py:
##########
@@ -200,6 +200,18 @@ def __radd__(self, other: Iterable[T]) -> Array[T]:
"""Concatenate two arrays."""
return type(self)(itertools.chain(other, self))
+ def __eq__(self, other: object) -> bool:
+ """Return True if the array is equal to another array."""
+ return _ffi_api.ArrayEqual(self, other)
+
+ def __ne__(self, other: object) -> bool:
+ """Return True if the array is not equal to another array."""
+ return not _ffi_api.ArrayEqual(self, other)
Review Comment:
The `__ne__` method can be removed. Python's default implementation for
`__ne__` delegates to `__eq__` and inverts the result, which is exactly what's
being done here. Removing this explicit implementation will make the code more
concise without changing its behavior.
##########
tests/python/test_container.py:
##########
@@ -227,3 +227,21 @@ def test_large_map_get() -> None:
def test_array_contains(arr: list[Any], value: Any, expected: bool) -> None:
a = tvm_ffi.convert(arr)
assert (value in a) == expected
+
+
[email protected](
+ "lhs, rhs, expected",
+ [
+ (tvm_ffi.Array([1, 2, 3]), tvm_ffi.Array([1, 2, 3]), True),
+ (tvm_ffi.Array([1, 2, 3]), tvm_ffi.Array([1, 2, 4]), False),
+ (tvm_ffi.Array([tvm_ffi.Array([1])]),
tvm_ffi.Array([tvm_ffi.Array([1])]), True),
+ (tvm_ffi.Array([tvm_ffi.Array([1])]),
tvm_ffi.Array([tvm_ffi.Array([2])]), False),
+ (tvm_ffi.Map({"a": 1, "b": 2}), tvm_ffi.Map({"a": 1, "b": 2}), True),
+ (tvm_ffi.Map({"a": 1, "b": 2}), tvm_ffi.Map({"a": 1, "b": 3}), False),
+ ],
Review Comment:
The current tests for container equality are a good start, but they could be
more comprehensive. I suggest adding more test cases to cover comparisons with
native Python containers (`list`, `dict`), order independence for maps,
handling of empty containers, nested containers, and comparisons between
different container types. This will ensure the new equality and hashing logic
is robust.
```python
[
# Array equality
(tvm_ffi.Array([1, 2, 3]), tvm_ffi.Array([1, 2, 3]), True),
(tvm_ffi.Array([1, 2, 3]), tvm_ffi.Array([1, 2, 4]), False),
(tvm_ffi.Array([1, 2, 3]), [1, 2, 3], True),
(tvm_ffi.Array([1, 2, 3]), [1, 2, 4], False),
(tvm_ffi.Array([]), tvm_ffi.Array([]), True),
(tvm_ffi.Array([]), [], True),
(tvm_ffi.Array([1]), [], False),
# Nested Array equality
(tvm_ffi.Array([tvm_ffi.Array([1])]),
tvm_ffi.Array([tvm_ffi.Array([1])]), True),
(tvm_ffi.Array([tvm_ffi.Array([1])]),
tvm_ffi.Array([tvm_ffi.Array([2])]), False),
(tvm_ffi.Array([[1]]), [[1]], True),
# Map equality
(tvm_ffi.Map({"a": 1, "b": 2}), tvm_ffi.Map({"a": 1, "b": 2}), True),
(tvm_ffi.Map({"a": 1, "b": 2}), tvm_ffi.Map({"b": 2, "a": 1}), True),
(tvm_ffi.Map({"a": 1, "b": 2}), tvm_ffi.Map({"a": 1, "b": 3}),
False),
(tvm_ffi.Map({"a": 1, "b": 2}), {"a": 1, "b": 2}, True),
(tvm_ffi.Map({"a": 1, "b": 2}), {"b": 2, "a": 1}, True),
(tvm_ffi.Map({"a": 1}), tvm_ffi.Map({"c": 1}), False),
(tvm_ffi.Map({"a": 1}), tvm_ffi.Map({"a": 1, "b": 2}), False),
(tvm_ffi.Map({}), tvm_ffi.Map({}), True),
(tvm_ffi.Map({}), {}, True),
(tvm_ffi.Map({1: 2}), {}, False),
# Nested Map equality
(tvm_ffi.Map({"a": {"b": 1}}), tvm_ffi.Map({"a": {"b": 1}}), True),
(tvm_ffi.Map({"a": {"b": 1}}), tvm_ffi.Map({"a": {"b": 2}}), False),
# Cross-type equality
(tvm_ffi.Array([1]), tvm_ffi.Map({1: 1}), False),
(tvm_ffi.Array([]), tvm_ffi.Map({}), False),
],
```
--
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]
