Copilot commented on code in PR #31:
URL: https://github.com/apache/tvm-ffi/pull/31#discussion_r2364518965
##########
python/tvm_ffi/cython/object.pxi:
##########
@@ -138,6 +138,16 @@ cdef class Object:
(<Object>fconstructor).chandle, <PyObject*>args, &chandle, NULL)
self.chandle = chandle
+ def __ffi_init__(self, *args) -> None:
+ """Initialize the instance using the ` __init__` method registered on
C++ side.
+
+ Parameters
+ ----------
+ args: list of objects
+ The arguments to the constructor
Review Comment:
[nitpick] Docstring has a stray space inside the backticks (` __init__`),
and describes args as 'list of objects' though the signature uses *args
(variadic positional arguments). Recommend: use `__init__` (no leading space)
and rephrase parameter section to reflect variadic usage, e.g. 'args: Any
Positional arguments forwarded to the registered C++ __init__'.
```suggestion
"""Initialize the instance using the `__init__` method registered on
the C++ side.
Parameters
----------
args : Any
Positional arguments forwarded to the registered C++ __init__.
```
##########
python/tvm_ffi/core.pyi:
##########
@@ -45,6 +45,15 @@ class Object:
def __ne__(self, other: Any) -> bool: ...
def __hash__(self) -> int: ...
def __init_handle_by_constructor__(self, fconstructor: Function, *args:
Any) -> None: ...
+ def __ffi_init__(self, *args: Any) -> None:
+ """Initialize the instance using the ` __init__` method registered on
C++ side.
+
+ Parameters
+ ----------
+ args: list of objects
+ The arguments to the constructor
Review Comment:
[nitpick] Same docstring issues as implementation: backticked name contains
an extra space (` __init__`) and parameter description should reflect variadic
positional arguments rather than 'list of objects'. Mirror the corrected
wording in the implementation for consistency.
```suggestion
"""Initialize the instance using the `__init__` method registered on
the C++ side.
Parameters
----------
args
Positional arguments to pass to the constructor.
```
##########
python/tvm_ffi/core.pyi:
##########
@@ -240,6 +249,7 @@ class TypeField:
frozen: bool
getter: Any
setter: Any
Review Comment:
[nitpick] New attribute dataclass_field is added without any doc/comment
explaining its purpose or when it may be None. Consider adding a brief
class-level note or inline comment to clarify its role (e.g., link to dataclass
reflection mechanism) to aid future maintainability.
```suggestion
setter: Any
# Reference to the corresponding dataclass field, if the FFI-backed type
is also a Python dataclass.
# May be None if no dataclass field is associated (e.g., for
non-dataclass types).
```
--
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]
