From 3228a7c8e40a5c6c41a9392026fe026703c50caa Mon Sep 17 00:00:00 2001 From: Richard Iannone Date: Wed, 22 Apr 2026 12:56:04 -0400 Subject: [PATCH 1/5] Handle dotted maturin names and None lineno --- great_docs/core.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/great_docs/core.py b/great_docs/core.py index 14713f1..b5da846 100644 --- a/great_docs/core.py +++ b/great_docs/core.py @@ -966,6 +966,14 @@ def _detect_module_name(self) -> str | None: # maturin projects: module name is often in module-name or derived from Cargo.toml module_name = maturin.get("module-name") if module_name: + # If `module-name` is a dotted path (e.g. "ggsql._ggsql"), + # it refers to the compiled extension submodule. The + # top-level importable Python package is the prefix — + # which is what we want for documentation. When + # `python-source` is set, the project also ships a + # pure-Python wrapper at that top-level name. + if "." in module_name: + return module_name.split(".", 1)[0] return module_name # Check [tool.setuptools.packages] for explicit package list @@ -6392,8 +6400,13 @@ def _categorize_api_objects(self, package_name: str, exports: list) -> dict: if member.kind.value in ("function", "method"): # Sub-classify to detect classmethod/staticmethod/property member_sub = self._sub_classify_function(member) - # Get line number for source ordering - lineno = getattr(member, "lineno", float("inf")) + # Get line number for source ordering. Dynamically inspected + # members (PyO3 / .so extensions) often have `lineno=None`; + # coerce to inf so sort() doesn't crash comparing + # `None < None`. + lineno = getattr(member, "lineno", None) + if lineno is None: + lineno = float("inf") # Validate with get_object if available if gd_get_object is not None: try: @@ -6570,7 +6583,11 @@ def _categorize_api_objects(self, package_name: str, exports: list) -> dict: if meth.kind.value in ("function", "method"): # Sub-classify for descriptor types meth_sub = self._sub_classify_function(meth) - lineno = getattr(meth, "lineno", float("inf")) + # `lineno` may be `None` for dynamically + # inspected (PyO3) methods. + lineno = getattr(meth, "lineno", None) + if lineno is None: + lineno = float("inf") if gd_get_object is not None: try: qd_m = gd_get_object( From 9011cc9cc7d8094ec8280022e3365ec1c54a6433 Mon Sep 17 00:00:00 2001 From: Richard Iannone Date: Wed, 22 Apr 2026 12:56:42 -0400 Subject: [PATCH 2/5] Handle compiled extensions and doc parsing errors --- great_docs/_renderer/_render/doc.py | 15 ++++++-- great_docs/_renderer/_rst_converters.py | 10 ++++++ great_docs/_renderer/blueprint.py | 34 ++++++++++++++++++ great_docs/_renderer/introspection.py | 47 +++++++++++++++++-------- 4 files changed, 90 insertions(+), 16 deletions(-) diff --git a/great_docs/_renderer/_render/doc.py b/great_docs/_renderer/_render/doc.py index f266b4a..66be902 100644 --- a/great_docs/_renderer/_render/doc.py +++ b/great_docs/_renderer/_render/doc.py @@ -582,9 +582,20 @@ def source_link(self) -> Link | None: if not base_url or base_url == "None": return None branch = package_info("GIT_REF") - relative_path = self.obj.relative_package_filepath + try: + relative_path = self.obj.relative_package_filepath + except (ValueError, AttributeError): + return None + # Suppress source links for compiled extensions (PyO3, Cython, + # pybind11, C extensions). The file path points at a binary + # artifact and `lineno` is None, producing broken `#LNone` anchors. + rel_str = str(relative_path) + if rel_str.endswith((".so", ".pyd", ".dylib", ".abi3.so")): + return None start, end = self.obj.lineno, self.obj.endlineno - anchor = f"#L{start}-L{end}" if start != end else f"#L{start}" + if start is None: + return None + anchor = f"#L{start}-L{end}" if end is not None and start != end else f"#L{start}" url = f"{base_url}/blob/{branch}/{relative_path}{anchor}" return Link("Source", url, attr=Attr(attributes={"target": "_blank", "rel": "noopener"})) diff --git a/great_docs/_renderer/_rst_converters.py b/great_docs/_renderer/_rst_converters.py index 477f357..87674ec 100644 --- a/great_docs/_renderer/_rst_converters.py +++ b/great_docs/_renderer/_rst_converters.py @@ -134,6 +134,16 @@ def _smart_dedent(text: str) -> str: def _convert_rst_text(text: str) -> str: """Apply all RST -> Markdown transforms to a docstring text section.""" + # Defensive coercion: some docstring section types (especially those produced + # by dynamically-inspected PyO3 modules or by section kinds without a + # dedicated singledispatch handler) may pass a non-string `el.value` here + # (e.g. a `list` of parameter / return entries). Rather than crashing the + # whole reference build with `AttributeError: 'list' object has no attribute + # 'splitlines'`, coerce to a string so the symbol still renders (with + # possibly degraded markup) and the rest of the page survives. + if not isinstance(text, str): + text = str(text) + # Fix docstrings where inspect.cleandoc failed to dedent (e.g. a # multiline string literal created a 0-indent line, preventing # proper margin detection). diff --git a/great_docs/_renderer/blueprint.py b/great_docs/_renderer/blueprint.py index 9a91450..8a07c77 100644 --- a/great_docs/_renderer/blueprint.py +++ b/great_docs/_renderer/blueprint.py @@ -19,6 +19,35 @@ from ._griffe import docstrings as ds from ._transformers import Node, PydanticTransformer, WorkaroundKeyError, ctx_node from .introspection import get_parser_defaults + +# Dunder members inherited from `object`/`type` (or PyO3 metaclasses) that carry docstrings but are +# never useful API documentation. Filtered out unconditionally so they do not pollute reference +# pages for compiled extensions (PyO3, Cython, pybind11, C extensions, etc.). +_BUILTIN_NOISE_DUNDERS = frozenset( + { + "__doc__", + "__module__", + "__dict__", + "__weakref__", + "__hash__", + "__class__", + "__class_getitem__", + "__init_subclass__", + "__subclasshook__", + "__match_args__", + "__slots__", + "__annotations__", + "__abstractmethods__", + "__dictoffset__", + "__flags__", + "__basicsize__", + "__itemsize__", + "__mro_entries__", + "__orig_bases__", + "__parameters__", + } +) + from .layout import ( MISSING, Auto, @@ -398,6 +427,11 @@ def _fetch_members(self, el: Auto, obj: dc.Object | dc.Alias) -> list[str]: options = obj.all_members if el.include_inherited else obj.members + # Always filter built-in noise dunders inherited from `object`/`type` + # (e.g. `__doc__`, `__module__`). These show up on every PyO3 / + # C-extension class because their docstrings are inherited from str. + options = {k: v for k, v in options.items() if k not in _BUILTIN_NOISE_DUNDERS} + if obj.is_module and obj.exports is not None: options = {k: v for k, v in options.items() if v.is_exported} diff --git a/great_docs/_renderer/introspection.py b/great_docs/_renderer/introspection.py index 1bbce5d..f34b0b0 100644 --- a/great_docs/_renderer/introspection.py +++ b/great_docs/_renderer/introspection.py @@ -257,7 +257,16 @@ def dynamic_alias(path: str, target: "str | None" = None, loader=None) -> dc.Obj if target: obj = get_object(target, loader=loader) else: - obj = get_object(canonical_path, loader=loader) + try: + obj = get_object(canonical_path, loader=loader) + except (KeyError, ModuleNotFoundError, ImportError): + # The canonical path computed via `__module__` doesn't refer to a + # loadable Python module, which is typical for PyO3 classes whose Rust + # `#[pyclass]` lacks `module = "..."` so `__module__` defaults + # to `"builtins"`. Fall back to the access path the user actually + # wrote, which by definition is importable. + obj = get_object(path, loader=loader) + canonical_path = path.replace(":", ".") replace_docstring(obj, attr) @@ -279,21 +288,31 @@ def dynamic_alias(path: str, target: "str | None" = None, loader=None) -> dc.Obj def _canonical_path(crnt_part: object, qualname: str) -> str | None: suffix = (":" + qualname) if qualname else "" - if not isinstance(crnt_part, ModuleType): - if inspect.isclass(crnt_part) or inspect.isfunction(crnt_part): - _mod = getattr(crnt_part, "__module__", None) + if isinstance(crnt_part, ModuleType): + return crnt_part.__name__ + suffix - if _mod is None: - return None - else: - qual_parts = [] if not qualname else qualname.split(".") - return _mod + ":" + ".".join([crnt_part.__qualname__, *qual_parts]) - elif isinstance(crnt_part, ModuleType): - return crnt_part.__name__ + suffix - else: + if inspect.isclass(crnt_part) or inspect.isfunction(crnt_part): + _mod = getattr(crnt_part, "__module__", None) + + if _mod is None: return None - else: - return crnt_part.__name__ + suffix + + qual_parts = [] if not qualname else qualname.split(".") + return _mod + ":" + ".".join([crnt_part.__qualname__, *qual_parts]) + + # PyO3 / C-extension callables (e.g. `builtin_function_or_method`, + # `method-wrapper`) don't satisfy `inspect.isfunction` but they do carry + # `__module__` / `__qualname__` attributes. Treat them as function-like + # so `dynamic_alias` can resolve them to their canonical home rather than + # building a self-referential alias on the re-exporting facade module + # (which produces `CyclicAliasError` downstream). + _mod = getattr(crnt_part, "__module__", None) + _qn = getattr(crnt_part, "__qualname__", None) + if _mod and _qn and callable(crnt_part): + qual_parts = [] if not qualname else qualname.split(".") + return _mod + ":" + ".".join([_qn, *qual_parts]) + + return None def _is_valueless(obj: dc.Object) -> bool: From 749c402749fafd83df316f67edd65a761848cd04 Mon Sep 17 00:00:00 2001 From: Richard Iannone Date: Wed, 22 Apr 2026 12:57:17 -0400 Subject: [PATCH 3/5] Add tests for PyO3-like renderer introspection --- tests/renderer/test_pyo3.py | 112 ++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 tests/renderer/test_pyo3.py diff --git a/tests/renderer/test_pyo3.py b/tests/renderer/test_pyo3.py new file mode 100644 index 0000000..dece7c3 --- /dev/null +++ b/tests/renderer/test_pyo3.py @@ -0,0 +1,112 @@ +"""Tests for PyO3 / C-extension robustness in the renderer's introspection. + +Real PyO3 packages aren't easy to fabricate inside the test suite, so these +tests use Python-level analogues that exhibit the same observable shape: + +* `builtin_function_or_method`: Python built-ins live in the `builtins` module and fail + `inspect.isfunction`, exactly like PyO3 `#[pyfunction]` exports re-exported through a Python + facade. +* `type` instances with `__module__ = "builtins"`: match PyO3 classes declared without + `#[pyclass(module = "...")]`. +""" + +from __future__ import annotations + +import sys +import types + +import pytest + +from great_docs._renderer.introspection import ( + _canonical_path, + dynamic_alias, +) + + +def test_canonical_path_handles_builtin_function(): + """`_canonical_path` should resolve PyO3-style builtins. + + `len` is a `builtin_function_or_method`: same C-level type as PyO3 `#[pyfunction]` exports. + `inspect.isfunction(len)` is `False`, so the pre-fix `_canonical_path` returned `None` and + `dynamic_alias` then built a self-referencing Alias on the facade module. + """ + result = _canonical_path(len, "") + assert result == "builtins:len" + + +def test_canonical_path_returns_none_for_plain_value(): + """Non-callable, non-module objects without __module__/__qualname__.""" + assert _canonical_path(42, "") is None + assert _canonical_path("hello", "") is None + + +def test_canonical_path_module(): + """Modules continue to resolve to their dotted name.""" + assert _canonical_path(sys, "") == "sys" + assert _canonical_path(sys, "path") == "sys:path" + + +def _make_facade_with_builtin(monkeypatch): + """Create a transient package whose facade re-exports a builtin function. + + Mirrors the ggsql pattern:: + + # ggsql/__init__.py + from ggsql._ggsql import execute + """ + pkg = types.ModuleType("_gd_pyo3_facade") + pkg.__path__ = [] # mark as package + # Re-export Python's built-in ``abs`` as if it were a PyO3 function. + pkg.execute = abs + monkeypatch.setitem(sys.modules, "_gd_pyo3_facade", pkg) + return pkg + + +def test_dynamic_alias_does_not_self_reference_pyo3_function(monkeypatch, tmp_path): + """Reproduce Issue 1: `dynamic_alias` must not build a cyclic Alias. + + Before the fix, calling `dynamic_alias` for a builtin re-exported through a package facade + produced an Alias whose `target` resolved back to the same path, then griffe raised + `CyclicAliasError` on first resolution. + """ + _make_facade_with_builtin(monkeypatch) + + # We don't have a real on-disk package for griffe to load, so just exercise + # the `_canonical_path` call inside dynamic_alias without expecting a fully + # resolved alias. The important behavioural check is that + # `_canonical_path` returns the *underlying* module path (`builtins:abs`) + # rather than `None` (which would cause the cyclic alias bug). + pkg = sys.modules["_gd_pyo3_facade"] + canonical = _canonical_path(pkg.execute, "") + assert canonical == "builtins:abs" + + +def test_convert_rst_text_tolerates_non_string(): + """Issue 5: a list-valued docstring section value must not crash rendering.""" + from great_docs._renderer._rst_converters import _convert_rst_text + + # A plain list (as produced by some docstring section kinds) should be + # coerced to a string instead of raising AttributeError. + out = _convert_rst_text(["a", "b"]) + assert isinstance(out, str) + # And a normal string still passes through transformations. + assert _convert_rst_text("hello") == "hello" + + +def test_lineno_none_does_not_crash_method_sort(): + """Issue 3: methods with `lineno=None` must sort without TypeError.""" + method_entries = [("foo", float("inf")), ("bar", float("inf"))] + # The fix coerces None -> inf so this comparison is valid. + method_entries.sort(key=lambda x: x[1]) + assert method_entries == [("foo", float("inf")), ("bar", float("inf"))] + + +@pytest.mark.parametrize( + "value", + [None, 42, ["a"], {"k": "v"}, ("t",)], +) +def test_convert_rst_text_handles_various_non_strings(value): + from great_docs._renderer._rst_converters import _convert_rst_text + + out = _convert_rst_text(value) + assert isinstance(out, str) From 2b1bb39ef8bcd2eeba7c1700352777ed6f2aa0ae Mon Sep 17 00:00:00 2001 From: Richard Iannone Date: Wed, 22 Apr 2026 12:57:33 -0400 Subject: [PATCH 4/5] Add ggsql-python (PyO3) to test packages --- test-packages/setup-test-packages.sh | 14 ++++++++++++++ tests/test_integration.py | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/test-packages/setup-test-packages.sh b/test-packages/setup-test-packages.sh index 80c8d8d..359a114 100755 --- a/test-packages/setup-test-packages.sh +++ b/test-packages/setup-test-packages.sh @@ -48,6 +48,20 @@ echo "================" # Setup py-shiny (large package, slower builds) setup_repo "https://github.com/posit-dev/py-shiny" "py-shiny" +echo "" +echo "PyO3 / Rust packages:" +echo "=====================" +# ggsql-python is a PyO3 (Rust) package — exercises Great Docs' handling +# of compiled extensions. Currently uses PR #2 branch (`great-docs`) which +# adds the docs setup; switch to `main` once merged. +setup_repo "https://github.com/posit-dev/ggsql-python" "ggsql-python" +(cd "$TEST_PACKAGES_DIR/ggsql-python" \ + && git fetch origin pull/2/head:great-docs 2>/dev/null || true) \ + && (cd "$TEST_PACKAGES_DIR/ggsql-python" && git checkout great-docs 2>/dev/null || true) +echo " Note: ggsql-python requires a Rust build. Install with:" +echo " pip install -e test-packages/ggsql-python" +echo "" + # Add more test packages here as needed # setup_repo "https://github.com/other/package" "package-name" diff --git a/tests/test_integration.py b/tests/test_integration.py index 359ea35..19b43ba 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -32,7 +32,7 @@ def _find_great_docs_executable(): # Listed in order of preference: smaller/faster packages first TEST_PACKAGES = [ pkg - for pkg in ["python-dateutil", "time-machine", "py-shiny"] + for pkg in ["python-dateutil", "time-machine", "py-shiny", "ggsql-python"] if (TEST_PACKAGES_DIR / pkg).exists() ] From 841cbe1986bcc7e9fc9457e28bf532e502bb4f84 Mon Sep 17 00:00:00 2001 From: Richard Iannone Date: Wed, 22 Apr 2026 13:28:06 -0400 Subject: [PATCH 5/5] Suppress redundant numpy docstring member sections --- great_docs/_renderer/_render/doc.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/great_docs/_renderer/_render/doc.py b/great_docs/_renderer/_render/doc.py index 66be902..dc494be 100644 --- a/great_docs/_renderer/_render/doc.py +++ b/great_docs/_renderer/_render/doc.py @@ -523,6 +523,21 @@ def _(self, el: qast.DocstringSectionSeeAlso): items.append((term, ":".join(desc))) return DefinitionList(items) + @render_docstring_section.register(gf.DocstringSectionFunctions) + @render_docstring_section.register(gf.DocstringSectionClasses) + @render_docstring_section.register(gf.DocstringSectionModules) + def _(self, el): + """ + Suppress collection-style sections (Methods, Functions, Classes, Modules, Attributes) + emitted by the numpy parser. + + These sections are hand-written summaries of class/module members (e.g., + `Methods\\n-------\\nfoo(x)\\n Description.`). Great Docs already auto-generates the same + data from the actual members, so rendering the docstring version produces redundant content. + Drop them silently rather than risking duplicate / out-of-sync tables. + """ + return None + @property def summary_name(self) -> str: """