Skip to content

Commit

Permalink
feat: Load private sibling modules by default when resolving aliases
Browse files Browse the repository at this point in the history
Before, external modules were only loaded to resolve aliases if the "resolve external" option/parameter was set to true. It meant that users had to explicitely set "external" to true to resolve aliases from `ast` to `_ast`, or to explicitely "preload" the private modules in mkdocstrings.

But `external=True` is a bit overpowered, and preloading private modules is a bit annoying.

Now, this external parameter accepts a new value, `None`, which also becomes the default value. When external is set to `None`, the loader will automatically load modules to resolve aliases if they are the private sibling of the originating module. It means that users can just load `ast` and be sure that only aliases pointing to `_ast` will be resolved.
  • Loading branch information
pawamoy committed May 6, 2024
1 parent ce1dcec commit 4806189
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 8 deletions.
16 changes: 13 additions & 3 deletions src/griffe/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def _load_packages(
docstring_options: dict[str, Any] | None = None,
resolve_aliases: bool = True,
resolve_implicit: bool = False,
resolve_external: bool = False,
resolve_external: bool | None = None,
allow_inspection: bool = True,
store_source: bool = True,
find_stubs_package: bool = False,
Expand Down Expand Up @@ -256,8 +256,17 @@ def add_subparser(command: str, text: str, **kwargs: Any) -> argparse.ArgumentPa
dump_options.add_argument(
"-U",
"--resolve-external",
dest="resolve_external",
action="store_true",
help="Whether to resolve aliases pointing to external/unknown modules (not loaded directly).",
help="Always resolve aliases pointing to external/unknown modules (not loaded directly)."
"Default is to resolve only from one module to its private sibling (`ast` -> `_ast`).",
)
dump_options.add_argument(
"--no-resolve-external",
dest="resolve_external",
action="store_false",
help="Never resolve aliases pointing to external/unknown modules (not loaded directly)."
"Default is to resolve only from one module to its private sibling (`ast` -> `_ast`).",
)
dump_options.add_argument(
"-S",
Expand Down Expand Up @@ -315,7 +324,7 @@ def dump(
extensions: Sequence[str | dict[str, Any] | ExtensionType | type[ExtensionType]] | None = None,
resolve_aliases: bool = False,
resolve_implicit: bool = False,
resolve_external: bool = False,
resolve_external: bool | None = None,
search_paths: Sequence[str | Path] | None = None,
find_stubs_package: bool = False,
append_sys_path: bool = False,
Expand All @@ -333,6 +342,7 @@ def dump(
resolve_aliases: Whether to resolve aliases (indirect objects references).
resolve_implicit: Whether to resolve every alias or only the explicitly exported ones.
resolve_external: Whether to load additional, unspecified modules to resolve aliases.
Default is to resolve only from one module to its private sibling (`ast` -> `_ast`).
extensions: The extensions to use.
search_paths: The paths to search into.
find_stubs_package: Whether to search for stubs-only packages.
Expand Down
10 changes: 5 additions & 5 deletions src/griffe/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def resolve_aliases(
self,
*,
implicit: bool = False,
external: bool = False,
external: bool | None = None,
max_iterations: int | None = None,
) -> tuple[set[str], int]:
"""Resolve aliases.
Expand Down Expand Up @@ -294,7 +294,7 @@ def expand_wildcards(
self,
obj: Object,
*,
external: bool = False,
external: bool | None = None,
seen: set | None = None,
) -> None:
"""Expand wildcards: try to recursively expand all found wildcards.
Expand All @@ -319,7 +319,7 @@ def expand_wildcards(

# Try loading the (unknown) package containing the wildcard importe module (if allowed to).
if not_loaded:
if not external:
if external is False or (external is None and package != f"_{obj.package.name}"):
continue
try:
self.load(package, try_relative_path=False)
Expand Down Expand Up @@ -407,7 +407,7 @@ def resolve_module_aliases(
obj: Object | Alias,
*,
implicit: bool = False,
external: bool = False,
external: bool | None = None,
seen: set[str] | None = None,
load_failures: set[str] | None = None,
) -> tuple[set[str], set[str]]:
Expand Down Expand Up @@ -448,7 +448,7 @@ def resolve_module_aliases(
unresolved.add(member.path)
package = target.split(".", 1)[0]
load_module = (
external
(external is True or (external is None and package == f"_{obj.package.name}"))
and package not in load_failures
and obj.package.path != package
and package not in self.modules_collection
Expand Down
28 changes: 28 additions & 0 deletions tests/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,3 +421,31 @@ def test_submodule_shadowing_member(init: str, caplog: pytest.LogCaptureFixture)
init=True,
):
assert "shadowing" in caplog.text


@pytest.mark.parametrize("wildcard", [True, False])
@pytest.mark.parametrize(("external", "foo_is_resolved"), [(None, True), (True, True), (False, False)])
def test_side_loading_sibling_private_module(wildcard: bool, external: bool | None, foo_is_resolved: bool) -> None:
"""Automatically load `_a` when `a` (wildcard) imports from it.
Parameters:
wildcard: Whether the import is a wildcard import.
external: Value for the `external` parameter when resolving aliases.
foo_is_resolved: Whether the `foo` alias should be resolved.
"""
with temporary_pypackage("_a", {"__init__.py": "def foo():\n '''Docstring.'''"}) as pkg_a: # noqa: SIM117
with temporary_pypackage("a", {"__init__.py": f"from _a import {'*' if wildcard else 'foo'}"}) as pkg_a_private:
loader = GriffeLoader(search_paths=[pkg_a.tmpdir, pkg_a_private.tmpdir])
package = loader.load("a")
loader.resolve_aliases(external=external, implicit=True)
if foo_is_resolved:
assert "foo" in package.members
assert package["foo"].is_alias
assert package["foo"].resolved
assert package["foo"].docstring.value == "Docstring."
elif wildcard:
assert "foo" not in package.members
else:
assert "foo" in package.members
assert package["foo"].is_alias
assert not package["foo"].resolved

0 comments on commit 4806189

Please sign in to comment.