gemini-code-assist[bot] commented on code in PR #52:
URL: https://github.com/apache/tvm-ffi/pull/52#discussion_r2373905653
##########
python/tvm_ffi/dataclasses/_utils.py:
##########
@@ -130,6 +130,18 @@ class DefaultFactory(NamedTuple):
cur_type_info = cur_type_info.parent_type_info
fields.reverse()
+ # init_fields: list[tuple[TypeField, Any]] = []
+ non_init_fields: list[tuple[str, Callable[[], Any]]] = []
+ # for field in fields:
+ # dataclass_field = field.dataclass_field
+ # assert field.name is not None
+ # if dataclass_field is None:
+ # raise ValueError(f"Missing dataclass metadata for field
`{field.name}`")
+ # if dataclass_field.init:
+ # init_fields.append((field, dataclass_field))
+ # elif (default_factory := dataclass_field.default_factory) is not
MISSING:
+ # non_init_fields.append((field.name, default_factory))
Review Comment:
This block contains commented-out code that appears to be a remnant from
development. To improve code clarity and maintainability, it should be removed,
keeping only the definition of `non_init_fields`.
```suggestion
non_init_fields: list[tuple[str, Callable[[], Any]]] = []
```
##########
python/tvm_ffi/dataclasses/field.py:
##########
@@ -95,9 +103,11 @@ class PyBase:
"""
if default is not MISSING and default_factory is not MISSING:
raise ValueError("Cannot specify both `default` and `default_factory`")
+ if not isinstance(init, bool):
+ raise TypeError("`init` must be a bool")
if default is not MISSING:
default_factory = _make_default_factory(default)
- ret = Field(default_factory=default_factory)
+ ret = Field(default_factory=default_factory, init=init)
return cast(_FieldValue, ret)
Review Comment:
Similar to Python's native `dataclasses`, it should be an error to specify a
field with `init=False` without providing a default value (`default` or
`default_factory`). Adding this check here will cause it to fail early with a
clear error message, improving robustness.
```suggestion
if default is not MISSING and default_factory is not MISSING:
raise ValueError("Cannot specify both `default` and
`default_factory`")
if not isinstance(init, bool):
raise TypeError("`init` must be a bool")
if not init and default is MISSING and default_factory is MISSING:
raise TypeError("field is init=False but has no default value")
if default is not MISSING:
default_factory = _make_default_factory(default)
ret = Field(default_factory=default_factory, init=init)
return cast(_FieldValue, ret)
```
--
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]
