gemini-code-assist[bot] commented on code in PR #177:
URL: https://github.com/apache/tvm-ffi/pull/177#discussion_r2445710937
##########
docs/conf.py:
##########
@@ -242,15 +244,70 @@ def setup(app: sphinx.application.Sphinx) -> None:
app.add_config_value("build_rust_docs", build_rust_docs, "env")
app.connect("config-inited", _apply_config_overrides)
app.connect("build-finished", _copy_rust_docs_to_output)
- app.connect("autodoc-skip-member", _never_skip_selected_dunders)
+ app.connect("autodoc-skip-member", _filter_inherited_members)
+ app.connect("autodoc-process-docstring", _link_inherited_members)
-def _never_skip_selected_dunders(app, what, name, obj, skip, options): #
noqa: ANN001, ANN202
+def _filter_inherited_members(app, what, name, obj, skip, options): # noqa:
ANN001, ANN202
if name in _autodoc_always_show:
- return False # do not skip
+ return False
+ if "built-in method " in str(obj):
+ # Skip: `str.maketrans`, `EnumType.from_bytes`
+ return True
+ if getattr(obj, "__objclass__", None) in _py_native_classes:
+ return True
return None
+def _link_inherited_members(app, what, name, obj, options, lines) -> None: #
noqa: ANN001
+ # Only act on members (methods/attributes/properties)
+ if what not in {"method", "attribute", "property"}:
+ return
+ cls = _import_cls(name.rsplit(".", 1)[0])
+ if cls is None:
+ return
+
+ member_name = name.rsplit(".", 1)[-1] # just "foo"
+ base = _defining_class(cls, member_name)
+
+ # If we can't find a base or this class defines it, nothing to do
+ if base is None or base is cls:
+ return
+
+ # If it comes from builtins we already hide it; no link needed
+ if base in _py_native_classes or getattr(base, "__module__", "") ==
"builtins":
+ return
+ owner_fq = f"{base.__module__}.{base.__qualname__}"
+ lines.clear()
+ lines.append(
+ f"*Defined in* :class:`~{owner_fq}` *as method*
:meth:`~{owner_fq}.{member_name}`."
+ )
Review Comment:
This code hardcodes "as method" and the `:meth:` role, which is incorrect
for attributes and properties. The `what` variable is checked to be one of
`{"method", "attribute", "property"}` at the beginning of the function. For
attributes and properties, you should use the `:attr:` role and adjust the text
accordingly. Properties are treated as attributes by Sphinx's autodoc.
```suggestion
role = "attr" if what in {"attribute", "property"} else "meth"
lines.clear()
lines.append(
f"*Defined in* :class:`~{owner_fq}` *as {what}*
:{role}:`~{owner_fq}.{member_name}`."
)
```
##########
docs/conf.py:
##########
@@ -242,15 +244,70 @@ def setup(app: sphinx.application.Sphinx) -> None:
app.add_config_value("build_rust_docs", build_rust_docs, "env")
app.connect("config-inited", _apply_config_overrides)
app.connect("build-finished", _copy_rust_docs_to_output)
- app.connect("autodoc-skip-member", _never_skip_selected_dunders)
+ app.connect("autodoc-skip-member", _filter_inherited_members)
+ app.connect("autodoc-process-docstring", _link_inherited_members)
-def _never_skip_selected_dunders(app, what, name, obj, skip, options): #
noqa: ANN001, ANN202
+def _filter_inherited_members(app, what, name, obj, skip, options): # noqa:
ANN001, ANN202
if name in _autodoc_always_show:
- return False # do not skip
+ return False
+ if "built-in method " in str(obj):
+ # Skip: `str.maketrans`, `EnumType.from_bytes`
+ return True
+ if getattr(obj, "__objclass__", None) in _py_native_classes:
+ return True
return None
+def _link_inherited_members(app, what, name, obj, options, lines) -> None: #
noqa: ANN001
+ # Only act on members (methods/attributes/properties)
+ if what not in {"method", "attribute", "property"}:
+ return
+ cls = _import_cls(name.rsplit(".", 1)[0])
+ if cls is None:
+ return
+
+ member_name = name.rsplit(".", 1)[-1] # just "foo"
+ base = _defining_class(cls, member_name)
+
+ # If we can't find a base or this class defines it, nothing to do
+ if base is None or base is cls:
+ return
+
+ # If it comes from builtins we already hide it; no link needed
+ if base in _py_native_classes or getattr(base, "__module__", "") ==
"builtins":
+ return
+ owner_fq = f"{base.__module__}.{base.__qualname__}"
+ lines.clear()
+ lines.append(
+ f"*Defined in* :class:`~{owner_fq}` *as method*
:meth:`~{owner_fq}.{member_name}`."
+ )
+
+
+def _defining_class(cls: type | None, attr_name: str) -> type | None:
+ """Find the first class in cls.__mro__ that defines attr_name in its
__dict__."""
+ if not isinstance(cls, type):
+ return None
+ method = getattr(cls, attr_name, None)
+ if method is None:
+ return None
+ for base in reversed(inspect.getmro(cls)):
+ d = getattr(base, "__dict__", {})
+ if d.get(attr_name, None) is method:
+ return base
+ return None
+
+
+def _import_cls(cls_name: str) -> type | None:
+ """Import and return the class object given its module and class name."""
+ try:
+ mod, clsname = cls_name.rsplit(".", 1)
+ m = importlib.import_module(mod)
+ return getattr(m, clsname, None)
+ except Exception:
Review Comment:
The `except Exception:` is too broad and can mask unexpected errors. It's
better to catch specific exceptions that you expect to happen, such as
`ImportError`, `AttributeError`, or `ValueError` (if `cls_name` doesn't contain
a '.'). This makes the code more robust and easier to debug.
```suggestion
except (ImportError, AttributeError, ValueError):
```
--
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]
