From 40c1f4952a79222fb2d5a58812e2de47209482ec Mon Sep 17 00:00:00 2001 From: sethg Date: Thu, 9 Oct 2025 13:51:45 +0200 Subject: [PATCH 1/6] Exit loop --- src/docstub/_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/docstub/_utils.py b/src/docstub/_utils.py index 1d1cb89..fe090d1 100644 --- a/src/docstub/_utils.py +++ b/src/docstub/_utils.py @@ -94,6 +94,7 @@ def module_name_from_path(path): if is_in_package: name_parts.insert(0, directory.name) directory = directory.parent + break else: break From fdb3fd5bfe1037f3e4bc7ea23676faddb66bd00f Mon Sep 17 00:00:00 2001 From: sethg Date: Fri, 10 Oct 2025 14:09:16 +0200 Subject: [PATCH 2/6] Stop at root --- src/docstub/_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/docstub/_utils.py b/src/docstub/_utils.py index fe090d1..96a2538 100644 --- a/src/docstub/_utils.py +++ b/src/docstub/_utils.py @@ -93,8 +93,9 @@ def module_name_from_path(path): is_in_package = (directory / "__init__.py").is_file() if is_in_package: name_parts.insert(0, directory.name) + if directory.parent == directory: + break directory = directory.parent - break else: break From b33fbb8f0b57c1ae22b81ab96808c6968419e2b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Gr=C3=BCter?= Date: Sat, 18 Oct 2025 15:24:03 +0200 Subject: [PATCH 3/6] Avoid endless loop for relative paths in `module_name_from_path` `Path(".").parent` can't move past the "." in a relative path and will just return `Path(".")` again. This lead to `while True` never breaking. To fix this, we make sure that absolute paths are used. We need to resolve the `path` before `lru_cache` sees it. Otherwise, `lru_cache` might return a wrong cached result in case the current working directory changes. That should never happen in docstub, but I think it's still good to be defensive here. This change also adds a few other defensive guards and asserts. Long term, it might be the least error-prone to resolve all paths to absolute ones as soon as possible. However, we'd have to do some additional work to shorten paths that are within the current working directory. Otherwise, users might see unnecessarily long paths in their output. --- src/docstub/_utils.py | 42 ++++++++++++++++++++++++++++++++++++++---- tests/test_utils.py | 42 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/src/docstub/_utils.py b/src/docstub/_utils.py index 96a2538..a10c91e 100644 --- a/src/docstub/_utils.py +++ b/src/docstub/_utils.py @@ -1,6 +1,6 @@ import itertools import re -from functools import lru_cache +from functools import lru_cache, wraps from zlib import crc32 @@ -60,6 +60,33 @@ def escape_qualname(name): return qualname +def _resolve_path_before_caching(func): + """Resolve relative paths passed to :func:`module_name_from_path`. + + :func:`module_name_from_path` makes use of Python's :func:`lru_cache` + decorator. Caching results based on relative paths may return wrong results + if the current working directory changes. + + Access the :func:`lru_cache` specific attributes with ``func.__wrapped__``. + + Parameters + ---------- + func : Callable + + Returns + ------- + wrapped : Callable + """ + + @wraps(func) + def wrapped(file_path): + file_path = file_path.resolve() + return func(file_path) + + return wrapped + + +@_resolve_path_before_caching @lru_cache(maxsize=100) def module_name_from_path(path): """Find the full name of a module within its package from its file path. @@ -86,18 +113,25 @@ def module_name_from_path(path): name_parts = [] if path.name != "__init__.py": + assert path.stem name_parts.insert(0, path.stem) + iter_limit = 10_000 directory = path.parent - while True: + for _ in range(iter_limit): is_in_package = (directory / "__init__.py").is_file() if is_in_package: + assert directory.name name_parts.insert(0, directory.name) - if directory.parent == directory: - break directory = directory.parent else: break + else: + msg = ( + f"Reached iteration limit ({iter_limit}) " + f"while trying to find module name for {path!r}" + ) + raise RuntimeError(msg) name = ".".join(name_parts) return name diff --git a/tests/test_utils.py b/tests/test_utils.py index bdd02c2..5bfec54 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,19 @@ +import os +from pathlib import Path + from docstub import _utils +def _create_dummy_package(root, structure): + """Create a dummy Python package in `root` based on subpaths in `structure`.""" + for item in structure: + path = root / item + if item.endswith(".py"): + path.touch() + else: + path.mkdir() + + class Test_module_name_from_path: def test_basic(self, tmp_path): # Package structure @@ -12,12 +25,7 @@ def test_basic(self, tmp_path): "foo/baz/__init__.py", "foo/baz/qux.py", ] - for item in structure: - path = tmp_path / item - if item.endswith(".py"): - path.touch() - else: - path.mkdir() + _create_dummy_package(tmp_path, structure) assert _utils.module_name_from_path(tmp_path / "foo/__init__.py") == "foo" assert _utils.module_name_from_path(tmp_path / "foo/bar.py") == "foo.bar" @@ -28,6 +36,28 @@ def test_basic(self, tmp_path): _utils.module_name_from_path(tmp_path / "foo/baz/qux.py") == "foo.baz.qux" ) + def test_relative_path(self, tmp_path_cwd): + structure = [ + "foo/", + "foo/__init__.py", + "foo/bar.py", + "foo/baz/", + "foo/baz/__init__.py", + "foo/baz/bar.py", + ] + _create_dummy_package(tmp_path_cwd, structure) + os.chdir(tmp_path_cwd / "foo") + cwd = Path() + + assert _utils.module_name_from_path(cwd / "__init__.py") == "foo" + assert _utils.module_name_from_path(cwd / "bar.py") == "foo.bar" + + # `./__init__.py` and `./bar.py` should return different results in + # different working directories + os.chdir(tmp_path_cwd / "foo/baz") + assert _utils.module_name_from_path(cwd / "__init__.py") == "foo.baz" + assert _utils.module_name_from_path(cwd / "bar.py") == "foo.baz.bar" + def test_pyfile_checksum(tmp_path): # Create package From ac8c34bc468e662ab0cca9cc305d0181e0d8e5e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Gr=C3=BCter?= Date: Sat, 18 Oct 2025 15:49:38 +0200 Subject: [PATCH 4/6] Warn in case docstub is run on a subpackage only --- src/docstub/_cli.py | 10 ++++++++-- src/docstub/_path_utils.py | 17 ++++++++++------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/docstub/_cli.py b/src/docstub/_cli.py index c8ebd70..77ee55c 100644 --- a/src/docstub/_cli.py +++ b/src/docstub/_cli.py @@ -18,6 +18,7 @@ from ._config import Config from ._path_utils import ( STUB_HEADER_COMMENT, + find_package_root, walk_source_and_targets, walk_source_package, ) @@ -326,8 +327,13 @@ def run( root_path = Path(root_path) if root_path.is_file(): logger.warning( - "Running docstub on a single file. Relative imports " - "or type references outside this file won't work." + "Running docstub on a single module. Relative imports " + "or type references pointing outside this module won't work." + ) + elif find_package_root(root_path) != root_path.resolve(): + logger.warning( + "Running docstub only on a subpackage. Relative imports " + "or type references pointing outside this subpackage won't work." ) config = _load_configuration(config_paths) diff --git a/src/docstub/_path_utils.py b/src/docstub/_path_utils.py index f1f9993..4737541 100644 --- a/src/docstub/_path_utils.py +++ b/src/docstub/_path_utils.py @@ -107,7 +107,7 @@ def is_python_package_dir(path): def find_package_root(path): - """Determine the root a Python package from any path pointing inside it. + """Determine the root of a Python package from any path pointing inside it. Parameters ---------- @@ -121,18 +121,21 @@ def find_package_root(path): -------- >>> from pathlib import Path >>> package_root = find_package_root(Path(__file__)) - >>> (package_root / "docstub").is_dir() + >>> package_root.name + 'docstub' + + >>> find_package_root(package_root) == package_root True """ - root = path - if root.is_file(): - root = root.parent + root = path.resolve() # `Path.parent` can't move past relative "." part for _ in range(2**16): - if not is_python_package_dir(root): + parent = root.parent + assert parent + if not is_python_package_dir(parent): logger.debug("Detected %s as the package root of %s", root, path) return root - root = root.parent + root = parent msg = f"exceeded iteration length while trying to find package root for {path}" raise RuntimeError(msg) From e16ba375ee6f876039580268e8b26c0ab5dad8a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Gr=C3=BCter?= Date: Sat, 18 Oct 2025 15:56:03 +0200 Subject: [PATCH 5/6] Appease mypy pointing out call to untyped function in tests --- tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 5bfec54..a15d9f3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,7 +4,7 @@ from docstub import _utils -def _create_dummy_package(root, structure): +def _create_dummy_package(root: Path, structure: list[str]): """Create a dummy Python package in `root` based on subpaths in `structure`.""" for item in structure: path = root / item From 05c6ac9b2bd35a59b4057782ebfe78b13fe0079c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Gr=C3=BCter?= Date: Sat, 18 Oct 2025 16:01:18 +0200 Subject: [PATCH 6/6] Appease mypy pointing out a missing return type annotation --- tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index a15d9f3..6b7b87c 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,7 +4,7 @@ from docstub import _utils -def _create_dummy_package(root: Path, structure: list[str]): +def _create_dummy_package(root: Path, structure: list[str]) -> None: """Create a dummy Python package in `root` based on subpaths in `structure`.""" for item in structure: path = root / item