gemini-code-assist[bot] commented on code in PR #146:
URL: https://github.com/apache/tvm-ffi/pull/146#discussion_r2436739412
##########
python/tvm_ffi/dataclasses/_utils.py:
##########
@@ -33,9 +33,9 @@
def type_info_to_cls(
type_info: TypeInfo,
- cls: type[_InputClsType],
+ cls: Type[_InputClsType], # noqa: UP006
methods: dict[str, Callable[..., Any] | None],
-) -> type[_InputClsType]:
+) -> Type[_InputClsType]: # noqa: UP006
Review Comment:
The `# noqa: UP006` comments here are incorrect. Ruff rule `UP006` is for
preferring `X | Y` over `Union[X, Y]`, which doesn't apply to `Type[...]`. The
change from `type` to `Type` is correct for Python 3.8 compatibility, and with
`target-version = "py38"` in your `ruff` configuration, it should not be
flagged. These `noqa` comments are misleading and should be removed.
```suggestion
cls: Type[_InputClsType],
methods: dict[str, Callable[..., Any] | None],
) -> Type[_InputClsType]:
```
##########
pyproject.toml:
##########
@@ -245,7 +247,7 @@ environment = { MACOSX_DEPLOYMENT_TARGET = "10.14" }
archs = ["AMD64"]
[tool.mypy]
-python_version = "3.8"
+python_version = "3.9"
Review Comment:
Since this package supports Python 3.8 (as indicated by `requires-python =
">=3.8"`), `mypy`'s `python_version` should be set to `"3.8"` to ensure that no
Python 3.9+ specific syntax is accidentally introduced. Setting it to `"3.9"`
could allow incompatible code to pass static checks, only to fail at runtime on
Python 3.8.
```suggestion
python_version = "3.8"
```
##########
python/tvm_ffi/dataclasses/c_class.py:
##########
@@ -114,7 +115,7 @@ class MyClass:
"""
- def decorator(super_type_cls: type[_InputClsType]) -> type[_InputClsType]:
+ def decorator(super_type_cls: Type[_InputClsType]) -> Type[_InputClsType]:
# noqa: UP006
Review Comment:
The `# noqa: UP006` comment is incorrect. Ruff rule `UP006` is for `Union` /
`Optional` syntax, not for `Type[...]`. This comment is misleading and should
be removed.
```suggestion
def decorator(super_type_cls: Type[_InputClsType]) ->
Type[_InputClsType]:
```
##########
tests/python/test_container.py:
##########
@@ -15,12 +15,21 @@
# specific language governing permissions and limitations
# under the License.
import pickle
+import sys
from collections.abc import Sequence
from typing import Any
import pytest
import tvm_ffi
+if sys.version_info >= (3, 9):
+ # PEP 585 generics
+ from collections.abc import Sequence
+else: # Python 3.8
+ # workarounds for python 3.8
+ # typing-module generics (subscriptable on 3.8)
+ from typing import Sequence
Review Comment:
This conditional import correctly handles `Sequence` for different Python
versions. However, `Sequence` is also imported unconditionally on line 19,
making that import redundant and potentially confusing. To improve clarity, the
import on line 19 should be removed, and your code should rely solely on this
conditional block.
##########
python/tvm_ffi/container.py:
##########
@@ -37,18 +33,38 @@
from . import _ffi_api, core
from .registry import register_object
-# workarounds for python 3.8
-if not (sys.version_info[0] == 3 and sys.version_info[1] <= 8):
- from typing import (
+if sys.version_info >= (3, 9):
+ # PEP 585 generics
+ from collections.abc import (
ItemsView as ItemsViewBase,
)
- from typing import (
+ from collections.abc import (
+ Iterable,
+ Iterator,
+ Mapping,
+ Sequence,
+ )
+ from collections.abc import (
KeysView as KeysViewBase,
)
+ from collections.abc import (
+ ValuesView as ValuesViewBase,
+ )
+else: # Python 3.8
+ # workarounds for python 3.8
+ # typing-module generics (subscriptable on 3.8)
from typing import (
+ ItemsView as ItemsViewBase,
+ )
+ from typing import (
+ Iterable,
+ Iterator,
Mapping,
Sequence,
)
+ from typing import (
+ KeysView as KeysViewBase,
+ )
from typing import (
ValuesView as ValuesViewBase,
)
Review Comment:
For better readability and to follow standard Python style, the imports
within the `if` and `else` blocks can be consolidated into single `from ...
import ...` statements.
```python
if sys.version_info >= (3, 9):
# PEP 585 generics
from collections.abc import (
ItemsView as ItemsViewBase,
Iterable,
Iterator,
KeysView as KeysViewBase,
Mapping,
Sequence,
ValuesView as ValuesViewBase,
)
else: # Python 3.8
# workarounds for python 3.8
# typing-module generics (subscriptable on 3.8)
from typing import (
ItemsView as ItemsViewBase,
Iterable,
Iterator,
KeysView as KeysViewBase,
Mapping,
Sequence,
ValuesView as ValuesViewBase,
)
```
##########
python/tvm_ffi/dataclasses/c_class.py:
##########
@@ -126,7 +127,7 @@ def decorator(super_type_cls: type[_InputClsType]) ->
type[_InputClsType]:
_utils.fill_dataclass_field(super_type_cls, type_field)
# Step 3. Create the proxy class with the fields as properties
fn_init = _utils.method_init(super_type_cls, type_info) if init else
None
- type_cls: type[_InputClsType] = _utils.type_info_to_cls(
+ type_cls: Type[_InputClsType] = _utils.type_info_to_cls( # noqa: UP006
Review Comment:
The `# noqa: UP006` comment is incorrect. Ruff rule `UP006` is for `Union` /
`Optional` syntax, not for `Type[...]`. This comment is misleading and should
be removed.
```suggestion
type_cls: Type[_InputClsType] = _utils.type_info_to_cls(
```
##########
python/tvm_ffi/dataclasses/c_class.py:
##########
@@ -41,7 +42,7 @@
@dataclass_transform(field_specifiers=(field,))
def c_class(
type_key: str, init: bool = True
-) -> Callable[[type[_InputClsType]], type[_InputClsType]]:
+) -> Callable[[Type[_InputClsType]], Type[_InputClsType]]: # noqa: UP006
Review Comment:
The `# noqa: UP006` comment is incorrect. Ruff rule `UP006` is for `Union` /
`Optional` syntax, not for `Type[...]`. This comment is misleading and should
be removed.
```suggestion
) -> Callable[[Type[_InputClsType]], Type[_InputClsType]]:
```
--
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]
