gemini-code-assist[bot] commented on PR #49:
URL: https://github.com/apache/tvm-ffi/pull/49#issuecomment-3335580445
## Summary of Changes
This pull request significantly enhances the Python FFI binding by
introducing automatic generation of Python classes for C++ object types that
lack explicit Python registration. This change aims to make the interaction
between C++ and Python more robust and user-friendly by reducing boilerplate.
It also includes a comprehensive correction of a pervasive typo in type
hierarchy fields across the codebase and refactors the attribute generation
logic for improved code reuse and maintainability.
### Highlights
* **Automatic Python Class Generation**: The system now automatically
generates Python classes for C++ object types that are not explicitly
registered in Python. This streamlines the process of exposing C++ objects to
the Python frontend, eliminating the need for manual registration of every C++
object type.
* **Typo Correction and Consistency**: A widespread typo, `type_acenstors`,
has been consistently corrected to `type_ancestors` across multiple C++ header
files, Cython declarations, and C++ source files, improving code readability
and correctness in type reflection mechanisms.
* **Refactored Attribute Generation Logic**: The logic for creating Python
properties (`as_property`) and callable methods (`as_callable`) from C++ FFI
type information has been centralized and is now reused for both explicitly
registered classes and the newly introduced auto-generated fallback classes.
* **Removal of Warnings for Unregistered Classes**: The previous behavior of
emitting a warning when an unregistered C++ object type was encountered has
been removed, aligning the system's behavior with `pybind` for a smoother user
experience.
* **Enhanced Python TypeInfo**: The Python `TypeInfo` class now explicitly
stores the `type_ancestors` information as a list of type indices, making the
inheritance hierarchy of C++ objects directly accessible and usable within
Python.
<details>
<summary><b>Changelog</b></summary>
* **include/tvm/ffi/c_api.h**
* Corrected `type_acenstors` to `type_ancestors` in the `TVMFFITypeInfo`
struct.
* **include/tvm/ffi/object.h**
* Corrected `type_acenstors` to `type_ancestors` within the
`IsObjectInstance` function.
* **include/tvm/ffi/reflection/accessor.h**
* Corrected `type_acenstors` to `type_ancestors` in `ForEachFieldInfo`
and `ForEachFieldInfoWithEarlyStop` functions.
* **python/tvm_ffi/core.pyi**
* Corrected a typo in the docstring for `__ffi_init__`.
* Updated type hints for `_set_type_cls` and renamed
`_lookup_type_info_from_type_key` to
`_lookup_or_register_type_info_from_type_key`.
* Added `as_callable` method to `TypeMethod` and `prototype_py` method
to `TypeInfo`.
* **python/tvm_ffi/cython/base.pxi**
* Corrected `type_acenstors` to `type_ancestors` and updated its type to
`const TVMFFITypeInfo**`.
* **python/tvm_ffi/cython/object.pxi**
* Added `from typing import Any` import.
* Introduced `make_fallback_cls_for_type_index` to dynamically create
Python classes for unregistered C++ types, including recursive parent class
creation.
* Modified `make_ret_object` to utilize the new fallback mechanism
instead of issuing a warning.
* Changed `_type_info_create_from_type_key` to a `cdef` function and
added logic to populate the `ancestors` list.
* Introduced `_update_registry` helper function for managing type
registrations.
* Modified `_register_object_by_index` to use the new `_update_registry`
function.
* Modified `_set_type_cls` to accept `type_info` directly and updated
internal assertions.
* Renamed `_lookup_type_info_from_type_key` to
`_lookup_or_register_type_info_from_type_key` and updated its logic to register
type info if not found.
* **python/tvm_ffi/cython/string.pxi**
* Registered `kTVMFFIObject` with the `Object` class.
* **python/tvm_ffi/cython/type_info.pxi**
* Added `from io import StringIO` import.
* Modified `TypeField.as_property` to use `cdef` for local variables and
simplify property assignment.
* Added `TypeMethod.as_callable` to correctly create Python method
attributes, handling both static and instance methods.
* Added `type_ancestors` to the `TypeInfo` dataclass and implemented
`__post_init__` to ensure parent type information is registered.
* Added `_member_method_wrapper` helper function.
* **python/tvm_ffi/dataclasses/_utils.py**
* Removed imports for `get_parent_type_info` and
`_lookup_type_info_from_type_key`.
* **python/tvm_ffi/dataclasses/c_class.py**
* Imported `_lookup_or_register_type_info_from_type_key` and
`_set_type_cls`.
* Removed `Field` from `field_specifiers` in the `dataclass_transform`
decorator.
* Modified the `decorator` function to use the new lookup/registration
mechanism and `_set_type_cls`, removing reliance on `get_parent_type_info`.
* **python/tvm_ffi/registry.py**
* Removed the `_member_method_wrapper` function.
* Refactored `_add_class_attrs` to leverage `field.as_property` and
`method.as_callable` for attribute assignment.
* **src/ffi/extra/reflection_extra.cc**
* Corrected `type_acenstors` to `type_ancestors` in
`MakeObjectFromPackedArgs`.
* **src/ffi/extra/testing.cc**
* Introduced `TestUnregisteredBaseObject` and `TestUnregisteredObject`
for testing the new fallback mechanism, including fields and methods.
* Registered the new test objects and their associated methods/fields.
* Updated `testing.make_unregistered_object` to create
`TestUnregisteredObject` with two arguments for testing.
* **src/ffi/object.cc**
* Corrected `type_acenstors_data` to `type_ancestors_data` and updated
its usage throughout the `TypeTable` class.
* **tests/cpp/test_object.cc**
* Corrected `type_acenstors` to `type_ancestors` in object type info
assertions.
* **tests/python/test_object.py**
* Imported `TypeInfo`.
* Modified `test_unregistered_object_fallback` to assert the properties
and methods of the auto-generated fallback class, and removed the
`pytest.warns` check for unregistered objects.
</details>
<details>
<summary><b>Activity</b></summary>
* The pull request was initially summarized by `gemini-code-assist[bot]`.
* The author, `junrushao`, requested reviews multiple times throughout the
development process.
* `junrushao` updated the PR based on offline discussions with `tqchen`,
specifically addressing `prototype_py` usage, warning removal, and refactoring
of `register_object` behavior.
* `junrushao` investigated a sporadic CI error related to
`TVMFFIGetTypeInfo` and `type_index=74`, identifying it as `OpaquePyObject` and
noting a potential fix in another PR.
* `gemini-code-assist[bot]` provided critical feedback on `prototype_py`'s
correctness, potential security risks, and issues with setting attributes on
`staticmethod` objects.
* `tqchen` suggested using Python's `type` function instead of `exec` for
class creation, which `junrushao` implemented.
* Discussions between `tqchen` and `junrushao` clarified the behavior of
registering fallback classes and the decision to remove warnings for
unregistered classes.
* `tqchen` raised concerns about reverse dependencies and the design choice
of making fallback classes dataclasses, suggesting alternative approaches.
* `tqchen` recommended renaming `_lookup_type_info_from_type_key` for better
clarity.
* `gemini-code-assist[bot]` identified a critical bug in
`make_fallback_cls_for_type_index` regarding recursive parent class creation
and provided a detailed explanation for the `staticmethod` attribute setting
issue after `junrushao` initially couldn't reproduce it.
</details>
--
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]
