python/apigen: Fix support for pybind11 overloaded functions

This was accidentally broken by
#357.

sphinx_immaterial/apidoc/python/apigen.py

@@ -119,10 +119,10 @@ class ParsedOverload(NamedTuple):
     overload_id: Optional[str] = None
     """Overload id specified in the docstring.
 
-  If there is just a single overload, will be `None`.  Otherwise, if no overload
-  id is specified, a warning is produced and the index of the overload,
-  i.e. "1", "2", etc., is used as the id.
-  """
+    If there is just a single overload, will be `None`.  Otherwise, if no overload
+    id is specified, a warning is produced and the index of the overload,
+    i.e. "1", "2", etc., is used as the id.
+    """
 
 
 def _extract_field(doc: str, field: str) -> Tuple[str, Optional[str]]:
@@ -1613,6 +1613,49 @@ def _summarize_rst_content(content: List[str]) -> List[str]:
     return content[:i]
 
 
+class _SiblingMatchKey(NamedTuple):
+    """Lookup key for finding siblings of a `_MemberDocumenterEntry`.
+
+    Normally siblings are determined based on the identity of the associated
+    Python object.
+
+    However, in the case of pybind11-overloaded functions, since all overloads
+    share the same Python object, the overload_id must also be taken into
+    account.
+    """
+
+    obj: Any
+    overload_id: Optional[str]
+
+    def __hash__(self):
+        return hash((id(self.obj), self.overload_id))
+
+    def __eq__(self, other):
+        if not isinstance(other, _SiblingMatchKey):
+            return False
+        return self.obj is other.obj and self.overload_id == other.overload_id
+
+
+def _get_sibling_match_key(entry: _MemberDocumenterEntry) -> Optional[_SiblingMatchKey]:
+    obj = None
+    if not isinstance(
+        entry.documenter,
+        (
+            sphinx.ext.autodoc.FunctionDocumenter,
+            sphinx.ext.autodoc.MethodDocumenter,
+            sphinx.ext.autodoc.PropertyDocumenter,
+        ),
+    ):
+        return None
+    obj = entry.documenter.object
+    overload_id: Optional[str] = None
+    overload = entry.overload
+    if overload is not None:
+        overload_id = overload.overload_id
+
+    return _SiblingMatchKey(obj, overload_id)
+
+
 class _ApiEntityCollector:
     def __init__(
         self,
@@ -1717,7 +1760,10 @@ def document_members(*args, **kwargs):
                         )
                     ]
         else:
-            signatures = entry.documenter.format_signature().split("\n")
+            if primary_entity is None:
+                signatures = entry.documenter.format_signature().split("\n")
+            else:
+                signatures = primary_entity.signatures
 
         overload_id: Optional[str] = None
         if entry.overload is not None:
@@ -1762,34 +1808,17 @@ def collect_documenter_members(
     ) -> List[_ApiEntityMemberReference]:
         members: List[_ApiEntityMemberReference] = []
 
-        object_to_api_entity_member_map: Dict[
-            int, Tuple[Any, _ApiEntityMemberReference]
-        ] = {}
+        sibling_match_map: dict[_SiblingMatchKey, _ApiEntityMemberReference] = {}
 
         for entry in _get_documenter_members(
             self.app, documenter, canonical_full_name=canonical_object_name
         ):
-            obj = None
-            if isinstance(
-                entry.documenter,
-                (
-                    sphinx.ext.autodoc.FunctionDocumenter,
-                    sphinx.ext.autodoc.MethodDocumenter,
-                    sphinx.ext.autodoc.PropertyDocumenter,
-                ),
-            ):
-                obj = entry.documenter.object
-            obj_and_primary_sibling_member: Optional[
-                Tuple[Any, _ApiEntityMemberReference]
-            ] = None
+            sibling_match_key = _get_sibling_match_key(entry)
             primary_sibling_entity: Optional[_ApiEntity] = None
             primary_sibling_member: Optional[_ApiEntityMemberReference] = None
-            if obj is not None:
-                obj_and_primary_sibling_member = object_to_api_entity_member_map.get(
-                    id(obj)
-                )
-                if obj_and_primary_sibling_member is not None:
-                    primary_sibling_member = obj_and_primary_sibling_member[1]
+            if (sibling_match_key := _get_sibling_match_key(entry)) is not None:
+                primary_sibling_member = sibling_match_map.get(sibling_match_key)
+                if primary_sibling_member is not None:
                     primary_sibling_entity = self.entities[
                         primary_sibling_member.canonical_object_name
                     ]
@@ -1816,8 +1845,8 @@ def collect_documenter_members(
                     member_canonical_object_name, True
                 )
             else:
-                if obj is not None:
-                    object_to_api_entity_member_map[id(obj)] = (obj, member)
+                if sibling_match_key is not None:
+                    sibling_match_map[sibling_match_key] = member
                 members.append(member)
 
             child.parents.append(member)

tests/issue_134/sphinx-immaterial-pybind11-issue-134/sphinx_immaterial_pybind11_issue_134.cpp

@@ -30,6 +30,24 @@ PYBIND11_MODULE(sphinx_immaterial_pybind11_issue_134, m) {
                               [](const Example& self) -> int { return 42; });
   }
 
+  cls.def(
+      "foo", [](Example& self, int x) { return 1; }, R"docstr(
+Int overload.
+
+Overload:
+  int
+)docstr");
+
+  cls.def(
+      "foo", [](Example& self, bool x) { return 1; }, R"docstr(
+Bool overload.
+
+Overload:
+  bool
+)docstr");
+
+  cls.attr("bar") = cls.attr("foo");
+
   cls.def_readonly("is_set_by_init", &Example::is_set_by_init, R"docstr(
             This read-only ``bool`` attribute is set by the constructor.
         )docstr");

tests/python_apigen_test.py

@@ -1,8 +1,10 @@
+import json
 import pathlib
 
 import pytest
 import sphinx
 
+
 from sphinx_immaterial.apidoc.python.apigen import _get_api_data
 
 if sphinx.version_info < (7, 2):
@@ -184,5 +186,41 @@ def test_type_params(apigen_make_app):
     )
     app.build()
     print(app._status.getvalue())
+    assert not app._warning.getvalue()
+
+
+def test_pybind11_overloaded_function(apigen_make_app, snapshot):
+    testmod = "sphinx_immaterial_pybind11_issue_134"
+    app = apigen_make_app(
+        confoverrides=dict(
+            python_apigen_modules={
+                testmod: "api/",
+            },
+        ),
+    )
+    print(app._status.getvalue())
     print(app._warning.getvalue())
     assert not app._warning.getvalue()
+
+    data = _get_api_data(app.env)
+
+    def get_entity_info(key):
+        entity = data.entities[key]
+        return json.dumps(
+            {
+                k: getattr(entity, k)
+                for k in ["signatures", "primary_entity", "siblings"]
+            },
+            indent=2,
+        )
+
+    for name in [
+        "Example.foo(int)",
+        "Example.foo(bool)",
+        "Example.bar(int)",
+        "Example.bar(bool)",
+    ]:
+        snapshot.assert_match(
+            get_entity_info(f"{testmod}.{name}"),
+            name.replace("(", "_").replace(")", "") + ".json",
+        )

tests/python_apigen_test_modules/pybind11_overloaded.py

@@ -0,0 +1,88 @@
+# fmt: off
+
+class Dim:
+    """1-d index interval with optionally-implicit bounds and dimension label.
+
+    Group:
+      Indexing
+    """
+
+    def __init__(self, *args, **kwargs) -> None:
+        """__init__(*args, **kwargs)
+Overloaded function.
+
+1. __init__(self: tensorstore_demo.Dim, label: Optional[str] = None, *, implicit_lower: bool = True, implicit_upper: bool = True) -> None
+
+
+Constructs an unbounded interval ``(-inf, +inf)``.
+
+Args:
+  label: Dimension label.
+  implicit_lower: Indicates whether the lower bound is
+    implicit.
+  implicit_upper: Indicates whether the upper bound is
+    implicit.
+
+Overload:
+  unbounded
+
+
+2. __init__(self: tensorstore_demo.Dim, size: Optional[int], label: Optional[str] = None, *, inclusive_min: Optional[int] = None, implicit_lower: bool = False, implicit_upper: Optional[bool] = None) -> None
+
+
+Constructs a sized interval ``[inclusive_min, inclusive_min+size)``.
+
+Args:
+  size: Size of the interval.
+  label: Dimension label.
+  inclusive_min: Inclusive lower bound.  Defaults to :python:`0`.
+  implicit_lower: Indicates whether the lower bound is
+    implicit.
+  implicit_upper: Indicates whether the upper bound is
+    implicit.  Defaults to :python:`False` if
+    :python:`size` is specified, otherwise :python:`True`.
+
+Overload:
+  size
+"""
+
+    def foo(self, *args, **kwargs) -> None:
+        """foo(*args, **kwargs)
+Overloaded function.
+
+1. foo(self: tensorstore_demo.Dim, label: Optional[str] = None, *, implicit_lower: bool = True, implicit_upper: bool = True) -> None
+
+
+Constructs an unbounded interval ``(-inf, +inf)``.
+
+Args:
+  label: Dimension label.
+  implicit_lower: Indicates whether the lower bound is
+    implicit.
+  implicit_upper: Indicates whether the upper bound is
+    implicit.
+
+Overload:
+  unbounded
+
+
+2. foo(self: tensorstore_demo.Dim, size: Optional[int], label: Optional[str] = None, *, inclusive_min: Optional[int] = None, implicit_lower: bool = False, implicit_upper: Optional[bool] = None) -> None
+
+
+Constructs a sized interval ``[inclusive_min, inclusive_min+size)``.
+
+Args:
+  size: Size of the interval.
+  label: Dimension label.
+  inclusive_min: Inclusive lower bound.  Defaults to :python:`0`.
+  implicit_lower: Indicates whether the lower bound is
+    implicit.
+  implicit_upper: Indicates whether the upper bound is
+    implicit.  Defaults to :python:`False` if
+    :python:`size` is specified, otherwise :python:`True`.
+
+Overload:
+  size
+"""
+
+    bar = foo

tests/snapshots/python_apigen_test/test_pybind11_overloaded_function/Example.bar_bool.json

@@ -0,0 +1,7 @@
+{
+  "signatures": [
+    "(self: sphinx_immaterial_pybind11_issue_134.Example, arg0: bool) -> int"
+  ],
+  "primary_entity": false,
+  "siblings": null
+}

tests/snapshots/python_apigen_test/test_pybind11_overloaded_function/Example.bar_int.json

@@ -0,0 +1,7 @@
+{
+  "signatures": [
+    "(self: sphinx_immaterial_pybind11_issue_134.Example, arg0: int) -> int"
+  ],
+  "primary_entity": false,
+  "siblings": null
+}

tests/snapshots/python_apigen_test/test_pybind11_overloaded_function/Example.foo_bool.json

@@ -0,0 +1,9 @@
+{
+  "signatures": [
+    "(self: sphinx_immaterial_pybind11_issue_134.Example, arg0: bool) -> int"
+  ],
+  "primary_entity": true,
+  "siblings": {
+    "sphinx_immaterial_pybind11_issue_134.Example.bar(bool)": true
+  }
+}

tests/snapshots/python_apigen_test/test_pybind11_overloaded_function/Example.foo_int.json

@@ -0,0 +1,9 @@
+{
+  "signatures": [
+    "(self: sphinx_immaterial_pybind11_issue_134.Example, arg0: int) -> int"
+  ],
+  "primary_entity": true,
+  "siblings": {
+    "sphinx_immaterial_pybind11_issue_134.Example.bar(int)": true
+  }
+}