Merge pull request #309 from machow/fix-dynamic-load-members

Fix dynamic load members

quartodoc/autosummary.py

@@ -283,11 +283,22 @@ def dynamic_alias(
 
         canonical_path = None
         crnt_part = mod
-        prev_part = _NoParent
         for ii, attr_name in enumerate(splits):
+            # update canonical_path ----
+            # this is our belief about where the final object lives (ie. its submodule)
+            try:
+                _qualname = ".".join(splits[ii:])
+                new_canonical_path = _canonical_path(crnt_part, _qualname)
+            except AttributeError:
+                new_canonical_path = None
+
+            if new_canonical_path is not None:
+                # Note that previously we kept the first valid canonical path,
+                # but now keep the last.
+                canonical_path = new_canonical_path
+
             # fetch attribute ----
             try:
-                prev_part = crnt_part
                 crnt_part = getattr(crnt_part, attr_name)
             except AttributeError:
                 # Fetching the attribute can fail if it is purely a type hint,
@@ -297,7 +308,6 @@ def dynamic_alias(
                     # See if we can return the static object for a value-less attr
                     try:
                         obj = get_object(canonical_path, loader=loader)
-                        print(obj)
                         if _is_valueless(obj):
                             return obj
                     except Exception as e:
@@ -308,21 +318,18 @@ def dynamic_alias(
                     f"No attribute named `{attr_name}` in the path `{path}`."
                 )
 
-            # update canonical_path ----
-            # this is our belief about where the final object lives (ie. its submodule)
-            try:
-                _qualname = ".".join(splits[ii:])
-                _is_final = ii == (len(splits) - 1)
-                new_canonical_path = _canonical_path(
-                    crnt_part, _qualname, _is_final, prev_part
-                )
-            except AttributeError:
-                new_canonical_path = None
+        # final canonical_path update ----
+        # TODO: this is largely identical to canonical_path update above
+        try:
+            _qualname = ""
+            new_canonical_path = _canonical_path(crnt_part, _qualname)
+        except AttributeError:
+            new_canonical_path = None
 
-            if new_canonical_path is not None:
-                # Note that previously we kept the first valid canonical path,
-                # but now keep the last.
-                canonical_path = new_canonical_path
+        if new_canonical_path is not None:
+            # Note that previously we kept the first valid canonical path,
+            # but now keep the last.
+            canonical_path = new_canonical_path
 
         if canonical_path is None:
             raise ValueError(f"Cannot find canonical path for `{path}`")
@@ -356,11 +363,8 @@ def dynamic_alias(
         return dc.Alias(attr_name, obj, parent=parent)
 
 
-class _NoParent:
-    """Represent the absence of a parent object."""
-
-
-def _canonical_path(crnt_part: object, qualname: str, is_final=False, parent=_NoParent):
+def _canonical_path(crnt_part: object, qualname: str):
+    suffix = (":" + qualname) if qualname else ""
     if not isinstance(crnt_part, ModuleType):
         # classes and functions ----
         if inspect.isclass(crnt_part) or inspect.isfunction(crnt_part):
@@ -371,19 +375,23 @@ def _canonical_path(crnt_part: object, qualname: str, is_final=False, parent=_No
             else:
                 # we can use the object's actual __qualname__ here, which correctly
                 # reports the path for e.g. methods on a class
-                return _mod + ":" + crnt_part.__qualname__
-        elif parent is not _NoParent and isinstance(parent, ModuleType):
-            return parent.__name__ + ":" + qualname
+                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:
             return None
-    elif isinstance(crnt_part, ModuleType) and is_final:
+    else:
         # final object is module
-        return crnt_part.__name__
+        return crnt_part.__name__ + suffix
 
 
 def _is_valueless(obj: dc.Object):
     if isinstance(obj, dc.Attribute):
-        if "class-attribute" in obj.labels and obj.value is None:
+        if (
+            obj.labels.union({"class-attribute", "module-attribute"})
+            and obj.value is None
+        ):
             return True
         elif "instance-attribute" in obj.labels:
             return True

quartodoc/tests/example_dynamic.py

@@ -3,6 +3,13 @@
 NOTE = "Notes\n----\nI am a note"
 
 
+a: int
+"""The a module attribute"""
+
+b: str = "2"
+"""The b module attribute"""
+
+
 def f(a, b, c):
     """Return something
 

quartodoc/tests/test_basic.py

@@ -1,7 +1,11 @@
+import pytest
+
 from quartodoc import get_object, get_function, MdRenderer
 from griffe.docstrings import dataclasses as ds
 from griffe import dataclasses as dc
 
+# TODO: rename to test_autosummary (or refactor autosummary into parts)
+
 
 def test_get_function():
     f_obj = get_function("quartodoc", "get_function")
@@ -89,12 +93,31 @@ def test_get_object_dynamic_class_instance_attr_doc():
     assert obj.members["b"].docstring.value == "The b attribute"
 
 
+@pytest.mark.xfail(reason="The object's docstring (str.__doc__) is currently used :/.")
+def test_get_object_dynamic_mod_instance_attr_doc():
+    obj = get_object("quartodoc.tests.example_dynamic:b", dynamic=True)
+
+    assert obj.docstring.value == "The b module attribute"
+
+
 def test_get_object_dynamic_class_instance_attr_doc_class_attr_valueless():
     obj = get_object("quartodoc.tests.example_dynamic:InstanceAttrs", dynamic=True)
 
     assert obj.members["z"].docstring.value == "The z attribute"
 
 
+def test_get_object_dynamic_mod_attr_valueless():
+    obj = get_object("quartodoc.tests.example_dynamic:a", dynamic=True)
+
+    assert obj.docstring.value == "The a module attribute"
+
+
+def test_get_object_dynamic_class_attr_valueless():
+    obj = get_object("quartodoc.tests.example_dynamic:InstanceAttrs.z", dynamic=True)
+
+    assert obj.docstring.value == "The z attribute"
+
+
 def test_get_object_dynamic_module_attr_str():
     # a key behavior here is that it does not error attempting to look up
     # str.__module__, which does not exist
@@ -129,3 +152,69 @@ def test_get_object_dynamic_class_method_assigned():
         obj.target.path
         == "quartodoc.tests.example_alias_target__nested.nested_alias_target"
     )
+
+
+def test_get_object_dynamic_toplevel_mod_attr(tmp_path):
+    """get_object with dynamic=True works for the top-level module's attributes"""
+    import sys
+
+    # TODO: should us a context handler
+    sys.path.insert(0, str(tmp_path))
+    (tmp_path / "some_mod.py").write_text(
+        '''
+a: int
+"""A module attribute"""
+'''
+    )
+
+    obj = get_object("some_mod:a", dynamic=True)
+    assert obj.docstring.value == "A module attribute"
+
+    sys.path.pop(sys.path.index(str(tmp_path)))
+
+
+@pytest.mark.parametrize(
+    "path,dst",
+    [
+        # No path returned, since it's ambiguous for an instance
+        # e.g. class location, vs instance location
+        ("quartodoc.tests.example:a_attr", None),
+        ("quartodoc.tests.example:AClass.a_attr", None),
+        # Functions give their submodule location
+        (
+            "quartodoc.tests.example:a_alias",
+            "quartodoc.tests.example_alias_target:alias_target",
+        ),
+        (
+            "quartodoc.tests.example:a_nested_alias",
+            "quartodoc.tests.example_alias_target__nested:nested_alias_target",
+        ),
+        (
+            "quartodoc.tests.example_alias_target:AClass.some_method",
+            "quartodoc.tests.example_alias_target__nested:nested_alias_target",
+        ),
+        # More mundane cases
+        ("quartodoc.tests.example", "quartodoc.tests.example"),
+        ("quartodoc.tests.example:a_func", "quartodoc.tests.example:a_func"),
+        ("quartodoc.tests.example:AClass", "quartodoc.tests.example:AClass"),
+        (
+            "quartodoc.tests.example:AClass.a_method",
+            "quartodoc.tests.example:AClass.a_method",
+        ),
+    ],
+)
+def test_func_canonical_path(path, dst):
+    import importlib
+    from quartodoc.autosummary import _canonical_path
+
+    mod_path, attr_path = path.split(":") if ":" in path else (path, "")
+
+    crnt_part = importlib.import_module(mod_path)
+
+    if attr_path:
+        for name in attr_path.split("."):
+            crnt_part = getattr(crnt_part, name)
+
+    res = _canonical_path(crnt_part, "")
+
+    assert res == dst