From 207c5f0fda50cf775bc1aeda2a5d3283a1ac1892 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Tue, 2 Dec 2025 17:23:32 +0000 Subject: [PATCH 01/21] Fix bug with FakeFileSystem Prior to this, subsequent sibling packages in the contents string would be incorrectly nested under the directory the line above, due to an assumption that the contents string would be dedented. This takes account of that. --- rust/src/filesystem.rs | 10 ++++------ tests/unit/adaptors/test_filesystem.py | 3 +++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/rust/src/filesystem.rs b/rust/src/filesystem.rs index c234aa64..8f3a74f0 100644 --- a/rust/src/filesystem.rs +++ b/rust/src/filesystem.rs @@ -323,7 +323,7 @@ fn parse_indented_file_system_string(file_system_string: &str) -> HashMap = HashMap::new(); let mut path_stack: Vec = Vec::new(); // Stores current directory path components let mut first_line = true; // Flag to handle the very first path component - + let mut first_line_indent: usize = 0; // Normalize newlines and split into lines let buffer = file_system_string.replace("\r\n", "\n"); let lines: Vec<&str> = buffer.split('\n').collect(); @@ -334,27 +334,25 @@ fn parse_indented_file_system_string(file_system_string: &str) -> HashMap current_depth { path_stack.pop(); } - // If the current line is a file, append it to the path for inserting into map, // then pop it off so that subsequent siblings are correctly handled. // If it's a directory, append it and it stays on the stack for its children. diff --git a/tests/unit/adaptors/test_filesystem.py b/tests/unit/adaptors/test_filesystem.py index e769902b..1c526bc2 100644 --- a/tests/unit/adaptors/test_filesystem.py +++ b/tests/unit/adaptors/test_filesystem.py @@ -40,6 +40,7 @@ def test_split(self, path, expected): ("/path/to/mypackage/readme.txt", True), ("/path/to/mypackage/foo/one.txt", True), ("/path/to/mypackage/foo/two/green.txt", True), + ("/path/to/mypackage/bar/blue.txt", True), ("/path/to/nonexistent.txt", False), ("/path/to/mypackage/purple.txt", False), ], @@ -53,6 +54,8 @@ def test_exists_content_only(self, file_name, expected): one.txt two/ green.txt + bar/ + blue.txt """ ) From 38d23e9a5ba421044b8593919accad3ae81a6e4b Mon Sep 17 00:00:00 2001 From: David Seddon Date: Wed, 3 Dec 2025 12:06:13 +0000 Subject: [PATCH 02/21] Correct wrong type being passed in test --- tests/unit/application/test_usecases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/application/test_usecases.py b/tests/unit/application/test_usecases.py index 5f7adbe8..94de9ee4 100644 --- a/tests/unit/application/test_usecases.py +++ b/tests/unit/application/test_usecases.py @@ -154,7 +154,7 @@ class FakePackageFinder(BaseFakePackageFinder): "mypackage", # Note: this should be a bool, but we want to tolerate it, # as Import Linter currently has a bug where it will pass it as None. - include_external_packages=None, + include_external_packages=False, ) expected_modules = { From 4362eed4066c592a02ef761284a39bec757f091a Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 14:58:35 +0000 Subject: [PATCH 03/21] Switch module_files to use Set As it's more flexible. --- src/grimp/application/ports/modulefinder.py | 3 +- tests/unit/adaptors/test_modulefinder.py | 52 +++++++++------------ tests/unit/application/test_scanning.py | 51 ++++++++++---------- 3 files changed, 48 insertions(+), 58 deletions(-) diff --git a/src/grimp/application/ports/modulefinder.py b/src/grimp/application/ports/modulefinder.py index bff66077..30fed153 100644 --- a/src/grimp/application/ports/modulefinder.py +++ b/src/grimp/application/ports/modulefinder.py @@ -1,4 +1,5 @@ import abc +from collections.abc import Set from dataclasses import dataclass from grimp.domain.valueobjects import Module @@ -20,7 +21,7 @@ class FoundPackage: name: str directory: str - module_files: frozenset[ModuleFile] + module_files: Set[ModuleFile] class AbstractModuleFinder(abc.ABC): diff --git a/tests/unit/adaptors/test_modulefinder.py b/tests/unit/adaptors/test_modulefinder.py index 9d7a04c1..33428e98 100644 --- a/tests/unit/adaptors/test_modulefinder.py +++ b/tests/unit/adaptors/test_modulefinder.py @@ -74,15 +74,13 @@ def test_namespaced_packages(): assert result == FoundPackage( name="somenamespace.foo", directory="/path/to/somenamespace/foo", - module_files=frozenset( - { - ModuleFile(module=Module("somenamespace.foo"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("somenamespace.foo.blue"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("somenamespace.foo.green"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("somenamespace.foo.green.one"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("somenamespace.foo.green.two"), mtime=DEFAULT_MTIME), - } - ), + module_files={ + ModuleFile(module=Module("somenamespace.foo"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.blue"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.green"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.green.one"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.green.two"), mtime=DEFAULT_MTIME), + }, ) @@ -114,13 +112,11 @@ def test_ignores_orphaned_python_files(): assert result == FoundPackage( name="mypackage", directory="/path/to/mypackage", - module_files=frozenset( - { - ModuleFile(module=Module("mypackage"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("mypackage.two"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("mypackage.two.green"), mtime=DEFAULT_MTIME), - } - ), + module_files={ + ModuleFile(module=Module("mypackage"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("mypackage.two"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("mypackage.two.green"), mtime=DEFAULT_MTIME), + }, ) @@ -155,13 +151,11 @@ def test_ignores_dotted_python_files(extension, should_warn, caplog): assert result == FoundPackage( name="mypackage", directory="/path/to/mypackage", - module_files=frozenset( - { - ModuleFile(module=Module("mypackage"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("mypackage.foo"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("mypackage.bar"), mtime=DEFAULT_MTIME), - } - ), + module_files={ + ModuleFile(module=Module("mypackage"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("mypackage.foo"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("mypackage.bar"), mtime=DEFAULT_MTIME), + }, ) if should_warn: assert caplog.messages == [ @@ -201,11 +195,9 @@ def test_ignores_hidden_directories(): assert result == FoundPackage( name="mypackage", directory="/path/to/mypackage", - module_files=frozenset( - { - ModuleFile(Module("mypackage"), mtime=DEFAULT_MTIME), - ModuleFile(Module("mypackage.two"), mtime=DEFAULT_MTIME), - ModuleFile(Module("mypackage.two.green"), mtime=DEFAULT_MTIME), - } - ), + module_files={ + ModuleFile(Module("mypackage"), mtime=DEFAULT_MTIME), + ModuleFile(Module("mypackage.two"), mtime=DEFAULT_MTIME), + ModuleFile(Module("mypackage.two.green"), mtime=DEFAULT_MTIME), + }, ) diff --git a/tests/unit/application/test_scanning.py b/tests/unit/application/test_scanning.py index af5fa125..bc8d1f6b 100644 --- a/tests/unit/application/test_scanning.py +++ b/tests/unit/application/test_scanning.py @@ -5,6 +5,7 @@ from grimp.domain.valueobjects import DirectImport, Module from tests.config import override_settings from grimp import _rustgrimp as rust # type: ignore[attr-defined] +from collections.abc import Set @pytest.mark.parametrize( @@ -175,7 +176,7 @@ def test_single_namespace_package_portion(): FoundPackage( name="namespace.foo", directory="/path/to/namespace/foo", - module_files=frozenset(_modules_to_module_files(all_modules)), + module_files=_modules_to_module_files(all_modules), ) } @@ -263,18 +264,16 @@ def test_import_of_portion_not_in_graph(include_external_packages): FoundPackage( name="namespace.foo", directory="/path/to/namespace/foo", - module_files=frozenset(_modules_to_module_files(all_modules)), + module_files=_modules_to_module_files(all_modules), ), FoundPackage( name="namespace.bar.one.green", directory="/path/to/namespace/bar/one/green", - module_files=frozenset( - _modules_to_module_files( - { - MODULE_BAR_ONE_GREEN, - MODULE_BAR_ONE_GREEN_ALPHA, - } - ) + module_files=_modules_to_module_files( + { + MODULE_BAR_ONE_GREEN, + MODULE_BAR_ONE_GREEN_ALPHA, + } ), ), } @@ -454,7 +453,7 @@ def test_absolute_from_imports(include_external_packages, expected_result): FoundPackage( name="foo", directory="/path/to/foo", - module_files=frozenset(_modules_to_module_files(all_modules)), + module_files=_modules_to_module_files(all_modules), ) } @@ -514,7 +513,7 @@ def test_relative_from_imports(module_to_scan_is_package): FoundPackage( name="foo", directory="/path/to/foo", - module_files=frozenset(_modules_to_module_files(all_modules)), + module_files=_modules_to_module_files(all_modules), ) } @@ -577,7 +576,7 @@ def test_trims_to_known_modules(import_source): FoundPackage( name="foo", directory="/path/to/foo", - module_files=frozenset(_modules_to_module_files(all_modules)), + module_files=_modules_to_module_files(all_modules), ) } @@ -631,7 +630,7 @@ def test_trims_to_known_modules_within_init_file(): FoundPackage( name="foo", directory="/path/to/foo", - module_files=frozenset(_modules_to_module_files(all_modules)), + module_files=_modules_to_module_files(all_modules), ) } @@ -684,7 +683,7 @@ def my_function(): FoundPackage( name="foo", directory="/path/to/foo", - module_files=frozenset(_modules_to_module_files(all_modules)), + module_files=_modules_to_module_files(all_modules), ) } @@ -752,14 +751,12 @@ def test_external_package_imports_for_namespace_packages(statement, expected_mod FoundPackage( name="namespace.foo.blue", directory="/path/to/namespace/foo/blue", - module_files=frozenset( - _modules_to_module_files( - { - Module("namespace.foo.blue"), - module_to_scan, - Module("namespace.foo.blue.beta"), - } - ) + module_files=_modules_to_module_files( + { + Module("namespace.foo.blue"), + module_to_scan, + Module("namespace.foo.blue.beta"), + } ), ) } @@ -807,12 +804,12 @@ def test_scans_multiple_packages(statement): FoundPackage( name="foo", directory="/path/to/foo", - module_files=frozenset(_modules_to_module_files(foo_modules)), + module_files=_modules_to_module_files(foo_modules), ), FoundPackage( name="bar", directory="/path/to/bar", - module_files=frozenset(_modules_to_module_files(bar_modules)), + module_files=_modules_to_module_files(bar_modules), ), } @@ -881,7 +878,7 @@ def test_exclude_type_checking_imports( FoundPackage( name="foo", directory="/path/to/foo", - module_files=frozenset(_modules_to_module_files(all_modules)), + module_files=_modules_to_module_files(all_modules), ) } @@ -983,5 +980,5 @@ def _module_to_module_file(module: Module) -> ModuleFile: return ModuleFile(module=module, mtime=some_mtime) -def _modules_to_module_files(modules: set[Module]) -> set[ModuleFile]: - return {_module_to_module_file(module) for module in modules} +def _modules_to_module_files(modules: Set[Module]) -> frozenset[ModuleFile]: + return frozenset({_module_to_module_file(module) for module in modules}) From c09464f4de23bba7b35fb7f791f36b9c00ae01d7 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 15:24:51 +0000 Subject: [PATCH 04/21] Prefactor test_walk --- tests/unit/adaptors/test_filesystem.py | 50 ++++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/tests/unit/adaptors/test_filesystem.py b/tests/unit/adaptors/test_filesystem.py index 1c526bc2..58cfa28d 100644 --- a/tests/unit/adaptors/test_filesystem.py +++ b/tests/unit/adaptors/test_filesystem.py @@ -1,3 +1,4 @@ +from typing import TypeAlias from copy import copy import pytest # type: ignore from grimp.application.ports.filesystem import BasicFileSystem @@ -159,10 +160,30 @@ def test_write(self): assert file_system.read(some_filename) == some_contents +WalkReturn: TypeAlias = tuple[str, list[str], list[str]] + + class TestFakeFileSystem(_Base): file_system_cls = FakeFileSystem - def test_walk(self): + MYPACKAGE: WalkReturn = ("/path/to/mypackage", ["foo"], ["__init__.py"]) + MYPACKAGE_FOO: WalkReturn = ("/path/to/mypackage/foo", ["two"], ["__init__.py", "one.py"]) + MYPACKAGE_FOO_TWO: WalkReturn = ( + "/path/to/mypackage/foo/two", + [], + ["__init__.py", "green.py", "blue.py"], + ) + + @pytest.mark.parametrize( + "directory, expected", + [ + ( + "/path/to/mypackage", + [MYPACKAGE, MYPACKAGE_FOO, MYPACKAGE_FOO_TWO], + ), + ], + ) + def test_walk(self, directory: str, expected: list[WalkReturn]): file_system = self.file_system_cls( """ /path/to/mypackage/ @@ -178,11 +199,10 @@ def test_walk(self): another.txt """ ) - assert [ - ("/path/to/mypackage", ["foo"], ["__init__.py"]), - ("/path/to/mypackage/foo", ["two"], ["__init__.py", "one.py"]), - ("/path/to/mypackage/foo/two", [], ["__init__.py", "green.py", "blue.py"]), - ] == list(file_system.walk("/path/to/mypackage")) + + result = list(file_system.walk(directory)) + + assert result == expected def test_empty_if_directory_does_not_exist(self): file_system = self.file_system_cls( @@ -208,15 +228,15 @@ def test_dirnames_can_be_modified_in_place(self): """ file_system = self.file_system_cls( """ - /path/to/mypackage/ - foo/ - one.txt - skipme/ - two.txt - dontskip/ - three.txt - bar/ - four.txt + /path/to/mypackage/ + foo/ + one.txt + skipme/ + two.txt + dontskip/ + three.txt + bar/ + four.txt """ ) From 5a76f4e0a80ec225e38bbb2b45f473623462b40a Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 16:09:21 +0000 Subject: [PATCH 05/21] Parse contents differently in FakeFileSystem This internal change will make it easier to walk the filesystem regardless of how deep the first line of contents is. --- tests/adaptors/filesystem.py | 143 +++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 41 deletions(-) diff --git a/tests/adaptors/filesystem.py b/tests/adaptors/filesystem.py index d8ed455d..4903ca11 100644 --- a/tests/adaptors/filesystem.py +++ b/tests/adaptors/filesystem.py @@ -68,12 +68,24 @@ def walk(self, directory_name): For each directory in the tree rooted at directory top (including top itself), it yields a 3-tuple (dirpath, dirnames, filenames). """ - try: - directory_contents = self.contents[directory_name] - except KeyError: + # Navigate through the nested structure to find the directory + directory_components = [c for c in directory_name.split("/") if c] + + contents = self.contents + for component in directory_components: + key_with_prefix = "/" + component + if key_with_prefix in contents: + contents = contents[key_with_prefix] + elif component in contents: + contents = contents[component] + else: + return [] + + # If we found a file (None) instead of a directory, return empty + if contents is None: return [] - yield from self._walk_contents(directory_contents, containing_directory=directory_name) + yield from self._walk_contents(contents, containing_directory=directory_name) def _walk_contents( self, directory_contents: dict[str, Any], containing_directory: str @@ -107,41 +119,85 @@ def split(self, file_name: str) -> tuple[str, str]: components.insert(0, "") return (self.sep.join(components[:-1]), components[-1]) - def _parse_contents(self, raw_contents: str | None): + def _parse_contents(self, raw_contents: str | None) -> dict[str, Any]: """ Returns the raw contents parsed in the form: { - '/path/to/mypackage': { - '__init__.py': None, - 'foo': { - '__init__.py': None, - 'one.py': None, - 'two': { - '__init__.py': None, - 'blue.py': None, - 'green.py': None, - } - } + "/path": { + "/to": + "/mypackage": { + "__init__.py": None, + "foo": { + "__init__.py": None, + "one.py": None, + "two": { + "__init__.py": None, + "blue.py": None, + "green.py": None, + } + }, + }, + }, } } """ if raw_contents is None: return {} - # Convert to yaml for ease of parsing. - yamlified_lines = [] raw_lines = [line for line in raw_contents.split("\n") if line.strip()] - dedented_lines = self._dedent(raw_lines) - for line in dedented_lines: - trimmed_line = line.rstrip().rstrip("/") - yamlified_line = trimmed_line + ":" - yamlified_lines.append(yamlified_line) - - yamlified_string = "\n".join(yamlified_lines) + # Group lines by their root paths + # A root path is a line that starts with "/" and has no indentation + root_path_groups = [] + current_group: list[str] = [] - return yaml.safe_load(yamlified_string) + for line in dedented_lines: + if line.startswith("/") and not line.startswith(" "): + # This is a new root path + if current_group: + root_path_groups.append(current_group) + current_group = [line] + else: + current_group.append(line) + + if current_group: + root_path_groups.append(current_group) + + # Process each root path group + result: dict[str, Any] = {} + for group in root_path_groups: + # First line is the root path + root_path = group[0].rstrip().rstrip("/") + path_components = [c for c in root_path.split("/") if c] + + # Remaining lines are the file tree + yamlified_lines = [] + for line in group[1:]: + trimmed_line = line.rstrip().rstrip("/") + yamlified_line = trimmed_line + ":" + yamlified_lines.append(yamlified_line) + + yamlified_string = "\n".join(yamlified_lines) + nested_contents = yaml.safe_load(yamlified_string) if yamlified_lines else {} + + # Build the nested structure from path components + group_result = nested_contents + for component in reversed(path_components): + group_result = {"/" + component: group_result} + + # Merge into result + self._deep_merge(result, group_result) + + return result + + def _deep_merge(self, target: dict, source: dict) -> None: + """Merge source dict into target dict recursively.""" + for key, value in source.items(): + if key in target and isinstance(target[key], dict) and isinstance(value, dict): + self._deep_merge(target[key], value) + else: + target[key] = value def _dedent(self, lines: list[str]) -> list[str]: """ @@ -167,23 +223,28 @@ def exists(self, file_name: str) -> bool: if file_name in self.content_map.keys(): return True - found_directory = None - for directory in self.contents.keys(): - if file_name.startswith(directory): - found_directory = directory - if not found_directory: - return False - - relative_file_name = file_name[len(found_directory) + 1 :] - file_components = relative_file_name.split("/") - - contents = self.contents[found_directory] - for component in file_components: - try: + # Split the file path into components and navigate through nested structure + file_components = [c for c in file_name.split("/") if c] + + contents = self.contents + for i, component in enumerate(file_components): + # First, try with "/" prefix (for the top-level path structure) + key_with_prefix = "/" + component + if key_with_prefix in contents: + contents = contents[key_with_prefix] + # If not found, try without prefix (for files and directories in the tree) + elif component in contents: contents = contents[component] - except KeyError: + else: return False - return True + + # If we've reached None, it's a file (leaf node) + if contents is None: + return True + + # If we've navigated through all components and haven't hit None, + # it's a directory, not a file - return False + return False def get_mtime(self, file_name: str) -> float: if not self.exists(file_name): From dd805dc3bb62104eadb2888b5b79d1450963bd53 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 16:11:30 +0000 Subject: [PATCH 06/21] Add test cases to show better file system walking --- tests/unit/adaptors/test_filesystem.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/unit/adaptors/test_filesystem.py b/tests/unit/adaptors/test_filesystem.py index 58cfa28d..27dbbf17 100644 --- a/tests/unit/adaptors/test_filesystem.py +++ b/tests/unit/adaptors/test_filesystem.py @@ -181,6 +181,18 @@ class TestFakeFileSystem(_Base): "/path/to/mypackage", [MYPACKAGE, MYPACKAGE_FOO, MYPACKAGE_FOO_TWO], ), + ( + "/path/to/mypackage/foo", + [MYPACKAGE_FOO, MYPACKAGE_FOO_TWO], + ), + ( + "/path/to/mypackage/foo/two", + [MYPACKAGE_FOO_TWO], + ), + ( + "/anotherpackage", + [("/anotherpackage", [], ["another.txt"])], + ), ], ) def test_walk(self, directory: str, expected: list[WalkReturn]): From 11c29436e9bee3151de4ceef9bff73a47f4c41fc Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 15:01:30 +0000 Subject: [PATCH 07/21] Prefactor test for adding more namespace tests --- tests/unit/adaptors/test_modulefinder.py | 68 ++++++++++++++++-------- 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/tests/unit/adaptors/test_modulefinder.py b/tests/unit/adaptors/test_modulefinder.py index 33428e98..ed27fee7 100644 --- a/tests/unit/adaptors/test_modulefinder.py +++ b/tests/unit/adaptors/test_modulefinder.py @@ -49,39 +49,65 @@ def test_happy_path(): ) -def test_namespaced_packages(): +MODULE_FILES_FOO_BLUE = { + ModuleFile(module=Module("somenamespace.foo.blue"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.blue.one"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.blue.two"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.blue.two.alpha"), mtime=DEFAULT_MTIME), +} +MODULE_FILES_FOO_GREEN_FIVE = { + ModuleFile(module=Module("somenamespace.foo.green"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.green.five"), mtime=DEFAULT_MTIME), + ModuleFile(module=Module("somenamespace.foo.green.five.beta"), mtime=DEFAULT_MTIME), +} + + +@pytest.mark.parametrize( + "package_name, package_directory, expected", + [ + ( + "somenamespace.foo.blue", + "/path/to/somenamespace/foo/blue", + FoundPackage( + name="somenamespace.foo.blue", + directory="/path/to/somenamespace/foo/blue", + module_files=MODULE_FILES_FOO_BLUE, + ), + ), + ], +) +def test_namespaced_packages(package_name: str, package_directory: str, expected: FoundPackage): module_finder = ModuleFinder() file_system = FakeFileSystem( contents=""" - /path/to/somenamespace/foo/ - __init__.py - blue.py - green/ - __init__.py - one.py - two/ + /path/to/somenamespace/ + foo/ + blue/ __init__.py + one.py + two/ + __init__.py + alpha.py + noinitpackage/ + three.py + orphan/ + __init__.py + four.py + green/ + five/ + __init__.py + beta.py """ ) result = module_finder.find_package( - package_name="somenamespace.foo", - package_directory="/path/to/somenamespace/foo", + package_name=package_name, + package_directory=package_directory, file_system=file_system, ) - assert result == FoundPackage( - name="somenamespace.foo", - directory="/path/to/somenamespace/foo", - module_files={ - ModuleFile(module=Module("somenamespace.foo"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("somenamespace.foo.blue"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("somenamespace.foo.green"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("somenamespace.foo.green.one"), mtime=DEFAULT_MTIME), - ModuleFile(module=Module("somenamespace.foo.green.two"), mtime=DEFAULT_MTIME), - }, - ) + assert result == expected def test_ignores_orphaned_python_files(): From b851f83efc1a497a796faac2e48d3faf69a65488 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 17:02:31 +0000 Subject: [PATCH 08/21] Add deterministic ordering to dataclasses This improves the debugging experience in IDEs. --- src/grimp/application/ports/modulefinder.py | 4 ++-- src/grimp/domain/analysis.py | 4 ++-- src/grimp/domain/valueobjects.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/grimp/application/ports/modulefinder.py b/src/grimp/application/ports/modulefinder.py index 30fed153..b427d816 100644 --- a/src/grimp/application/ports/modulefinder.py +++ b/src/grimp/application/ports/modulefinder.py @@ -7,13 +7,13 @@ from .filesystem import AbstractFileSystem -@dataclass(frozen=True) +@dataclass(frozen=True, order=True) class ModuleFile: module: Module mtime: float -@dataclass(frozen=True) +@dataclass(frozen=True, order=True) class FoundPackage: """ Set of modules found under a single package, together with metadata. diff --git a/src/grimp/domain/analysis.py b/src/grimp/domain/analysis.py index 3c529c38..88a54179 100644 --- a/src/grimp/domain/analysis.py +++ b/src/grimp/domain/analysis.py @@ -4,7 +4,7 @@ from collections.abc import Iterable, Sequence -@dataclass(frozen=True) +@dataclass(frozen=True, order=True) class Route: """ A set of 'chains' that share the same middle. @@ -63,7 +63,7 @@ def single_chained(cls, *modules: str) -> Route: ) -@dataclass(frozen=True) +@dataclass(frozen=True, order=True) class PackageDependency: """ Dependencies from one package to another. diff --git a/src/grimp/domain/valueobjects.py b/src/grimp/domain/valueobjects.py index ec2e64e9..66bb60fb 100644 --- a/src/grimp/domain/valueobjects.py +++ b/src/grimp/domain/valueobjects.py @@ -1,7 +1,7 @@ from dataclasses import dataclass -@dataclass(frozen=True) +@dataclass(frozen=True, order=True) class Module: """ A Python module. @@ -42,7 +42,7 @@ def is_descendant_of(self, module: "Module") -> bool: return self.name.startswith(f"{module.name}.") -@dataclass(frozen=True) +@dataclass(frozen=True, order=True) class DirectImport: """ An import between one module and another. @@ -57,7 +57,7 @@ def __str__(self) -> str: return f"{self.importer} -> {self.imported} (l. {self.line_number})" -@dataclass(frozen=True) +@dataclass(frozen=True, order=True) class Layer: """ A layer within a layered architecture. From cff77f79135471b37f162afd49321a10f0e5aae1 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 17:15:02 +0000 Subject: [PATCH 09/21] Allow passing namespace packages to ModuleFinder --- src/grimp/adaptors/modulefinder.py | 25 +++++++++++++++++------- tests/unit/adaptors/test_modulefinder.py | 19 +++++++++++++++++- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/grimp/adaptors/modulefinder.py b/src/grimp/adaptors/modulefinder.py index 934190f9..9433333e 100644 --- a/src/grimp/adaptors/modulefinder.py +++ b/src/grimp/adaptors/modulefinder.py @@ -1,5 +1,5 @@ import logging -from collections.abc import Iterable +from collections.abc import Iterable, Set from grimp.application.ports import modulefinder from grimp.application.ports.filesystem import AbstractFileSystem @@ -37,13 +37,21 @@ def _get_python_files_inside_package(self, directory: str) -> Iterable[str]: Return: Generator of Python file names. """ + portion_dirs: set[str] = set() + for dirpath, dirs, files in self.file_system.walk(directory): - # Don't include directories that aren't Python packages, - # nor their subdirectories. - if "__init__.py" not in files: - for d in list(dirs): - dirs.remove(d) - continue + if self._is_in_portion(dirpath, portion_dirs): + # Are we somewhere inside a non-namespace package? + if "__init__.py" not in files: + # Don't drill down further in this directory. + # (This means we won't include 'orphans' - Python packages deeply nested + # in a package that has already included __init__.py files. + for d in list(dirs): + dirs.remove(d) + continue + elif "__init__.py" in files: + # This directory is a portion (i.e. it has a top-level __init__.py). + portion_dirs.add(dirpath) # Don't include hidden directories. dirs_to_remove = [d for d in dirs if self._should_ignore_dir(d)] @@ -54,6 +62,9 @@ def _get_python_files_inside_package(self, directory: str) -> Iterable[str]: if self._is_python_file(filename, dirpath): yield self.file_system.join(dirpath, filename) + def _is_in_portion(self, directory: str, portions: Set[str]) -> bool: + return any(directory.startswith(portion) for portion in portions) + def _should_ignore_dir(self, directory: str) -> bool: # TODO: make this configurable. # Skip adding directories that are hidden. diff --git a/tests/unit/adaptors/test_modulefinder.py b/tests/unit/adaptors/test_modulefinder.py index ed27fee7..da36c8b7 100644 --- a/tests/unit/adaptors/test_modulefinder.py +++ b/tests/unit/adaptors/test_modulefinder.py @@ -56,7 +56,6 @@ def test_happy_path(): ModuleFile(module=Module("somenamespace.foo.blue.two.alpha"), mtime=DEFAULT_MTIME), } MODULE_FILES_FOO_GREEN_FIVE = { - ModuleFile(module=Module("somenamespace.foo.green"), mtime=DEFAULT_MTIME), ModuleFile(module=Module("somenamespace.foo.green.five"), mtime=DEFAULT_MTIME), ModuleFile(module=Module("somenamespace.foo.green.five.beta"), mtime=DEFAULT_MTIME), } @@ -65,6 +64,24 @@ def test_happy_path(): @pytest.mark.parametrize( "package_name, package_directory, expected", [ + ( + "somenamespace", + "/path/to/somenamespace", + FoundPackage( + name="somenamespace", + directory="/path/to/somenamespace", + module_files=MODULE_FILES_FOO_BLUE | MODULE_FILES_FOO_GREEN_FIVE, + ), + ), + ( + "somenamespace.foo", + "/path/to/somenamespace/foo", + FoundPackage( + name="somenamespace.foo", + directory="/path/to/somenamespace/foo", + module_files=MODULE_FILES_FOO_BLUE | MODULE_FILES_FOO_GREEN_FIVE, + ), + ), ( "somenamespace.foo.blue", "/path/to/somenamespace/foo/blue", From ba6e8288cfa3a58809618d6bd70bc83d20845fc9 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 17:46:13 +0000 Subject: [PATCH 10/21] Include namespace_packages in FoundPackage --- src/grimp/adaptors/modulefinder.py | 51 ++++++++++++++++++--- src/grimp/application/ports/modulefinder.py | 1 + tests/unit/adaptors/test_modulefinder.py | 13 ++++++ 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/grimp/adaptors/modulefinder.py b/src/grimp/adaptors/modulefinder.py index 9433333e..22f1a256 100644 --- a/src/grimp/adaptors/modulefinder.py +++ b/src/grimp/adaptors/modulefinder.py @@ -16,7 +16,10 @@ def find_package( module_files: list[modulefinder.ModuleFile] = [] - for module_filename in self._get_python_files_inside_package(package_directory): + python_files, namespace_dirs = self._get_python_files_and_namespace_dirs_inside_package( + package_directory + ) + for module_filename in python_files: module_name = self._module_name_from_filename( package_name, module_filename, package_directory ) @@ -25,18 +28,33 @@ def find_package( modulefinder.ModuleFile(module=Module(module_name), mtime=module_mtime) ) + namespace_packages = frozenset( + { + self._namespace_from_dir(package_name, namespace_dir, package_directory) + for namespace_dir in namespace_dirs + } + ) + return modulefinder.FoundPackage( name=package_name, directory=package_directory, module_files=frozenset(module_files), + namespace_packages=namespace_packages, ) - def _get_python_files_inside_package(self, directory: str) -> Iterable[str]: + def _get_python_files_and_namespace_dirs_inside_package( + self, directory: str + ) -> tuple[Iterable[str], Set[str]]: """ - Get a list of Python files within the supplied package directory. - Return: - Generator of Python file names. + Search the supplied package directory for Python files and namespaces. + + Return tuple consisting of: + 1. Iterable of Python file names. + 2. Set of namespace directories encountered. """ + python_files: list[str] = [] + namespace_dirs: set[str] = set() + portion_dirs: set[str] = set() for dirpath, dirs, files in self.file_system.walk(directory): @@ -52,6 +70,8 @@ def _get_python_files_inside_package(self, directory: str) -> Iterable[str]: elif "__init__.py" in files: # This directory is a portion (i.e. it has a top-level __init__.py). portion_dirs.add(dirpath) + else: + namespace_dirs.add(dirpath) # Don't include hidden directories. dirs_to_remove = [d for d in dirs if self._should_ignore_dir(d)] @@ -60,7 +80,9 @@ def _get_python_files_inside_package(self, directory: str) -> Iterable[str]: for filename in files: if self._is_python_file(filename, dirpath): - yield self.file_system.join(dirpath, filename) + python_files.append(self.file_system.join(dirpath, filename)) + + return python_files, namespace_dirs def _is_in_portion(self, directory: str, portions: Set[str]) -> bool: return any(directory.startswith(portion) for portion in portions) @@ -118,3 +140,20 @@ def _module_name_from_filename( if components[-1] == "__init__": components.pop() return ".".join(components) + + def _namespace_from_dir( + self, package_name: str, namespace_dir: str, package_directory: str + ) -> str: + """ + Args: + package_name (string) - the importable name of the top level package. Could + be namespaced. + namespace_dir (string) - the full name of the namespace directory. + package_directory (string) - the full path of the top level Python package directory. + Returns: + Absolute module name for importing (string). + """ + parent_of_package_directory = package_directory[: -len(package_name)] + directory_relative_to_parent = namespace_dir[len(parent_of_package_directory) :] + components = directory_relative_to_parent.split(self.file_system.sep) + return ".".join(components) diff --git a/src/grimp/application/ports/modulefinder.py b/src/grimp/application/ports/modulefinder.py index b427d816..779e6b7e 100644 --- a/src/grimp/application/ports/modulefinder.py +++ b/src/grimp/application/ports/modulefinder.py @@ -22,6 +22,7 @@ class FoundPackage: name: str directory: str module_files: Set[ModuleFile] + namespace_packages: Set[str] = frozenset() class AbstractModuleFinder(abc.ABC): diff --git a/tests/unit/adaptors/test_modulefinder.py b/tests/unit/adaptors/test_modulefinder.py index da36c8b7..97d3edb8 100644 --- a/tests/unit/adaptors/test_modulefinder.py +++ b/tests/unit/adaptors/test_modulefinder.py @@ -71,6 +71,13 @@ def test_happy_path(): name="somenamespace", directory="/path/to/somenamespace", module_files=MODULE_FILES_FOO_BLUE | MODULE_FILES_FOO_GREEN_FIVE, + namespace_packages=frozenset( + { + "somenamespace", + "somenamespace.foo", + "somenamespace.foo.green", + } + ), ), ), ( @@ -80,6 +87,12 @@ def test_happy_path(): name="somenamespace.foo", directory="/path/to/somenamespace/foo", module_files=MODULE_FILES_FOO_BLUE | MODULE_FILES_FOO_GREEN_FIVE, + namespace_packages=frozenset( + { + "somenamespace.foo", + "somenamespace.foo.green", + } + ), ), ), ( From cc94114016cfa6272d8a43aef9b836bac0cce568 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Tue, 2 Dec 2025 17:56:12 +0000 Subject: [PATCH 11/21] Make package finder return multiple directories This doesn't change the implementation yet, but it prepares us for supporting namespace packages passed in. --- src/grimp/adaptors/packagefinder.py | 7 ++++++- src/grimp/application/ports/packagefinder.py | 4 ++-- src/grimp/application/usecases.py | 10 +++++++++- tests/adaptors/packagefinder.py | 6 +++--- tests/unit/adaptors/test_packagefinder.py | 18 +++++++++--------- 5 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/grimp/adaptors/packagefinder.py b/src/grimp/adaptors/packagefinder.py index e87f644e..4115b60f 100644 --- a/src/grimp/adaptors/packagefinder.py +++ b/src/grimp/adaptors/packagefinder.py @@ -12,7 +12,7 @@ class ImportLibPackageFinder(AbstractPackageFinder): - def determine_package_directory( + def _determine_package_directory( self, package_name: str, file_system: AbstractFileSystem ) -> str: # TODO - do we need to add the current working directory here? @@ -34,6 +34,11 @@ def determine_package_directory( "adding an __init__.py file should fix the problem." ) + def determine_package_directories( + self, package_name: str, file_system: AbstractFileSystem + ) -> set[str]: + return {self._determine_package_directory(package_name, file_system)} + def _is_a_package(self, spec: ModuleSpec, file_system: AbstractFileSystem) -> bool: assert spec.origin filename = file_system.split(spec.origin)[1] diff --git a/src/grimp/application/ports/packagefinder.py b/src/grimp/application/ports/packagefinder.py index 09c4da45..30b6d0ed 100644 --- a/src/grimp/application/ports/packagefinder.py +++ b/src/grimp/application/ports/packagefinder.py @@ -5,7 +5,7 @@ class AbstractPackageFinder(abc.ABC): @abc.abstractmethod - def determine_package_directory( + def determine_package_directories( self, package_name: str, file_system: AbstractFileSystem - ) -> str: + ) -> set[str]: raise NotImplementedError diff --git a/src/grimp/application/usecases.py b/src/grimp/application/usecases.py index e7503d29..7f544863 100644 --- a/src/grimp/application/usecases.py +++ b/src/grimp/application/usecases.py @@ -3,6 +3,7 @@ """ from typing import cast + from collections.abc import Sequence, Iterable from .scanning import scan_imports @@ -12,6 +13,7 @@ from ..application.ports.modulefinder import AbstractModuleFinder, FoundPackage, ModuleFile from ..application.ports.packagefinder import AbstractPackageFinder from ..domain.valueobjects import DirectImport, Module +from .. import exceptions from .config import settings @@ -79,9 +81,15 @@ def _find_packages( found_packages: set[FoundPackage] = set() for package_name in package_names: - package_directory = package_finder.determine_package_directory( + package_directories = package_finder.determine_package_directories( package_name=package_name, file_system=file_system ) + try: + [package_directory] = package_directories + except ValueError: + # TODO handle multiple directories. + raise exceptions.NamespacePackageEncountered + found_package = module_finder.find_package( package_name=package_name, package_directory=package_directory, diff --git a/tests/adaptors/packagefinder.py b/tests/adaptors/packagefinder.py index b242f26d..a44fe4d3 100644 --- a/tests/adaptors/packagefinder.py +++ b/tests/adaptors/packagefinder.py @@ -5,7 +5,7 @@ class BaseFakePackageFinder(AbstractPackageFinder): directory_map: dict[str, str] = {} - def determine_package_directory( + def determine_package_directories( self, package_name: str, file_system: AbstractFileSystem - ) -> str: - return self.directory_map[package_name] + ) -> set[str]: + return {self.directory_map[package_name]} diff --git a/tests/unit/adaptors/test_packagefinder.py b/tests/unit/adaptors/test_packagefinder.py index 51bb2eed..63494c6e 100644 --- a/tests/unit/adaptors/test_packagefinder.py +++ b/tests/unit/adaptors/test_packagefinder.py @@ -14,20 +14,20 @@ @pytest.mark.parametrize( "package, expected", ( - ("testpackage", assets / "testpackage"), + ("testpackage", {str(assets / "testpackage")}), ( "mynamespace.green", - assets / "namespacepackages" / "locationone" / "mynamespace" / "green", + {str(assets / "namespacepackages" / "locationone" / "mynamespace" / "green")}, ), ( "mynamespace.blue", - assets / "namespacepackages" / "locationtwo" / "mynamespace" / "blue", + {str(assets / "namespacepackages" / "locationtwo" / "mynamespace" / "blue")}, ), ), ) -def test_determine_package_directory(package, expected): - assert ImportLibPackageFinder().determine_package_directory(package, FileSystem()) == str( - expected +def test_determine_package_directories(package, expected): + assert ( + ImportLibPackageFinder().determine_package_directories(package, FileSystem()) == expected ) @@ -40,7 +40,7 @@ def test_determine_package_directory_doesnt_support_namespace_packages(): "namespace packages, adding an __init__.py file should fix the problem." ), ): - ImportLibPackageFinder().determine_package_directory("mynamespace", FakeFileSystem()) + ImportLibPackageFinder().determine_package_directories("mynamespace", FakeFileSystem()) @pytest.mark.parametrize( @@ -53,8 +53,8 @@ def test_determine_package_directory_doesnt_support_namespace_packages(): "mynamespace.yellow", ), ) -def test_determine_package_directory_doesnt_support_non_top_level_modules(package): +def test_determine_package_directories_doesnt_support_non_top_level_modules(package): with pytest.raises( exceptions.NotATopLevelModule, ): - ImportLibPackageFinder().determine_package_directory(package, FakeFileSystem()) + ImportLibPackageFinder().determine_package_directories(package, FakeFileSystem()) From 143a531a7ed4d73db487ef5e444c2887af7d5b4f Mon Sep 17 00:00:00 2001 From: David Seddon Date: Wed, 3 Dec 2025 10:46:16 +0000 Subject: [PATCH 12/21] Return all directories from namespace packages This pushes the NamespacePackageEncountered error up the call stack into the use case. --- src/grimp/adaptors/packagefinder.py | 19 +++----------- tests/functional/test_error_handling.py | 13 --------- tests/unit/adaptors/test_packagefinder.py | 32 ++++++++++++++--------- 3 files changed, 23 insertions(+), 41 deletions(-) diff --git a/src/grimp/adaptors/packagefinder.py b/src/grimp/adaptors/packagefinder.py index 4115b60f..55c5d28f 100644 --- a/src/grimp/adaptors/packagefinder.py +++ b/src/grimp/adaptors/packagefinder.py @@ -12,10 +12,9 @@ class ImportLibPackageFinder(AbstractPackageFinder): - def _determine_package_directory( + def determine_package_directories( self, package_name: str, file_system: AbstractFileSystem - ) -> str: - # TODO - do we need to add the current working directory here? + ) -> set[str]: # Attempt to locate the package file. spec = importlib.util.find_spec(package_name) if not spec: @@ -26,18 +25,8 @@ def _determine_package_directory( if not self._is_a_package(spec, file_system) or self._has_a_non_namespace_parent(spec): raise exceptions.NotATopLevelModule - return file_system.dirname(spec.origin) - - raise exceptions.NamespacePackageEncountered( - f"Package '{package_name}' is a namespace package (see PEP 420). Try specifying the " - "portion name instead. If you are not intentionally using namespace packages, " - "adding an __init__.py file should fix the problem." - ) - - def determine_package_directories( - self, package_name: str, file_system: AbstractFileSystem - ) -> set[str]: - return {self._determine_package_directory(package_name, file_system)} + assert spec.submodule_search_locations # This should be the case if spec.has_location. + return set(spec.submodule_search_locations) def _is_a_package(self, spec: ModuleSpec, file_system: AbstractFileSystem) -> bool: assert spec.origin diff --git a/tests/functional/test_error_handling.py b/tests/functional/test_error_handling.py index 45635986..e79914ba 100644 --- a/tests/functional/test_error_handling.py +++ b/tests/functional/test_error_handling.py @@ -1,5 +1,4 @@ import os -import re import pytest # type: ignore @@ -19,15 +18,3 @@ def test_syntax_error_includes_module(): filename=filename, lineno=5, text="fromb . import two" ) assert expected_exception == excinfo.value - - -def test_missing_root_init_file(): - with pytest.raises( - exceptions.NamespacePackageEncountered, - match=re.escape( - "Package 'missingrootinitpackage' is a namespace package (see PEP 420). Try specifying " - "the portion name instead. If you are not intentionally " - "using namespace packages, adding an __init__.py file should fix the problem." - ), - ): - build_graph("missingrootinitpackage", cache_dir=None) diff --git a/tests/unit/adaptors/test_packagefinder.py b/tests/unit/adaptors/test_packagefinder.py index 63494c6e..f7f2de62 100644 --- a/tests/unit/adaptors/test_packagefinder.py +++ b/tests/unit/adaptors/test_packagefinder.py @@ -1,4 +1,3 @@ -import re from pathlib import Path import pytest # type: ignore @@ -15,6 +14,18 @@ "package, expected", ( ("testpackage", {str(assets / "testpackage")}), + ( + "missingrootinitpackage", + { + str(assets / "missingrootinitpackage"), + }, + ), + ( + "missingrootinitpackage.one", + { + str(assets / "missingrootinitpackage" / "one"), + }, + ), ( "mynamespace.green", {str(assets / "namespacepackages" / "locationone" / "mynamespace" / "green")}, @@ -23,6 +34,13 @@ "mynamespace.blue", {str(assets / "namespacepackages" / "locationtwo" / "mynamespace" / "blue")}, ), + ( + "mynamespace", + { + str(assets / "namespacepackages" / "locationone" / "mynamespace"), + str(assets / "namespacepackages" / "locationtwo" / "mynamespace"), + }, + ), ), ) def test_determine_package_directories(package, expected): @@ -31,18 +49,6 @@ def test_determine_package_directories(package, expected): ) -def test_determine_package_directory_doesnt_support_namespace_packages(): - with pytest.raises( - exceptions.NamespacePackageEncountered, - match=re.escape( - "Package 'mynamespace' is a namespace package (see PEP 420). Try specifying the" - " portion name instead. If you are not intentionally using " - "namespace packages, adding an __init__.py file should fix the problem." - ), - ): - ImportLibPackageFinder().determine_package_directories("mynamespace", FakeFileSystem()) - - @pytest.mark.parametrize( "package", ( From b17128c2186df8989c357a7b11b2e03c1e6b83ba Mon Sep 17 00:00:00 2001 From: David Seddon Date: Wed, 3 Dec 2025 11:31:38 +0000 Subject: [PATCH 13/21] Adjust BaseFakePackageFinder to support multiple directories --- tests/adaptors/packagefinder.py | 6 ++++-- tests/unit/application/test_usecases.py | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/adaptors/packagefinder.py b/tests/adaptors/packagefinder.py index a44fe4d3..5740677d 100644 --- a/tests/adaptors/packagefinder.py +++ b/tests/adaptors/packagefinder.py @@ -1,11 +1,13 @@ +from collections.abc import Mapping from grimp.application.ports.packagefinder import AbstractPackageFinder + from grimp.application.ports.filesystem import AbstractFileSystem class BaseFakePackageFinder(AbstractPackageFinder): - directory_map: dict[str, str] = {} + directory_map: Mapping[str, set[str]] = {} def determine_package_directories( self, package_name: str, file_system: AbstractFileSystem ) -> set[str]: - return {self.directory_map[package_name]} + return self.directory_map[package_name] diff --git a/tests/unit/application/test_usecases.py b/tests/unit/application/test_usecases.py index 94de9ee4..3922ce35 100644 --- a/tests/unit/application/test_usecases.py +++ b/tests/unit/application/test_usecases.py @@ -36,7 +36,7 @@ def test_happy_path(self, include_external_packages): ) class FakePackageFinder(BaseFakePackageFinder): - directory_map = {"mypackage": "/path/to/mypackage"} + directory_map = {"mypackage": {"/path/to/mypackage"}} with override_settings(FILE_SYSTEM=file_system, PACKAGE_FINDER=FakePackageFinder()): graph = usecases.build_graph( @@ -85,7 +85,7 @@ def test_build_graph_respects_cache_dir(self, supplied_cache_dir): file_system = FakeFileSystem() class FakePackageFinder(BaseFakePackageFinder): - directory_map = {"mypackage": "/path/to/mypackage"} + directory_map = {"mypackage": {"/path/to/mypackage"}} SOME_DEFAULT_CACHE_DIR = ".some_default" @@ -147,7 +147,7 @@ def test_forgives_wrong_type_being_passed_to_include_external_packages(self): ) class FakePackageFinder(BaseFakePackageFinder): - directory_map = {"mypackage": "/path/to/mypackage"} + directory_map = {"mypackage": {"/path/to/mypackage"}} with override_settings(FILE_SYSTEM=file_system, PACKAGE_FINDER=FakePackageFinder()): graph = usecases.build_graph( From 53bc1c95ce9d63a6bb62d6f1bc8b436f8b5e8cd7 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Thu, 4 Dec 2025 18:16:00 +0000 Subject: [PATCH 14/21] Allow passing in namespace packages to build_graph This gets us most of the way, but we still want to include the namespace packages in the graph too (depending on what's passed in). --- src/grimp/application/usecases.py | 21 ++++----- tests/functional/test_namespace_packages.py | 52 ++++++++++++++++----- tests/unit/application/test_usecases.py | 45 ++++++++++++++++++ 3 files changed, 94 insertions(+), 24 deletions(-) diff --git a/src/grimp/application/usecases.py b/src/grimp/application/usecases.py index 7f544863..8341a449 100644 --- a/src/grimp/application/usecases.py +++ b/src/grimp/application/usecases.py @@ -13,7 +13,6 @@ from ..application.ports.modulefinder import AbstractModuleFinder, FoundPackage, ModuleFile from ..application.ports.packagefinder import AbstractPackageFinder from ..domain.valueobjects import DirectImport, Module -from .. import exceptions from .config import settings @@ -84,18 +83,14 @@ def _find_packages( package_directories = package_finder.determine_package_directories( package_name=package_name, file_system=file_system ) - try: - [package_directory] = package_directories - except ValueError: - # TODO handle multiple directories. - raise exceptions.NamespacePackageEncountered - - found_package = module_finder.find_package( - package_name=package_name, - package_directory=package_directory, - file_system=file_system, - ) - found_packages.add(found_package) + for package_directory in package_directories: + found_package = module_finder.find_package( + package_name=package_name, + package_directory=package_directory, + file_system=file_system, + ) + found_packages.add(found_package) + return found_packages diff --git a/tests/functional/test_namespace_packages.py b/tests/functional/test_namespace_packages.py index 0d0d8b83..697fc6cc 100644 --- a/tests/functional/test_namespace_packages.py +++ b/tests/functional/test_namespace_packages.py @@ -1,6 +1,6 @@ import pytest # type: ignore -from grimp import build_graph, exceptions +from grimp import build_graph """ For ease of reference, these are the imports of all the files: @@ -17,14 +17,16 @@ nestednamespace.foo.alpha.blue.one, nestednamespace.bar.beta.orange """ +YELLOW_MODULES = {"mynamespace.yellow"} +GREEN_MODULES = {"mynamespace.green", "mynamespace.green.alpha"} +BLUE_MODULES = {"mynamespace.blue", "mynamespace.blue.alpha", "mynamespace.blue.beta"} -def test_build_graph_for_namespace(): - with pytest.raises(exceptions.NamespacePackageEncountered): - build_graph("mynamespace", cache_dir=None) +def test_build_graph_for_namespace(): + graph = build_graph("mynamespace", cache_dir=None) -GREEN_MODULES = {"mynamespace.green", "mynamespace.green.alpha"} -BLUE_MODULES = {"mynamespace.blue", "mynamespace.blue.alpha", "mynamespace.blue.beta"} + assert graph.modules == YELLOW_MODULES | GREEN_MODULES | BLUE_MODULES + assert graph.count_imports() @pytest.mark.parametrize( @@ -99,14 +101,42 @@ def test_import_between_namespace_children(): # Nested namespaces +FOO_ALPHA_BLUE_MODULES = { + "nestednamespace.foo.alpha.blue", + "nestednamespace.foo.alpha.blue.one", + "nestednamespace.foo.alpha.blue.two", +} +FOO_ALPHA_GREEN_MODULES = { + "nestednamespace.foo.alpha.green", + "nestednamespace.foo.alpha.green.one", + "nestednamespace.foo.alpha.green.two", +} +BAR_BETA_MODULES = { + "nestednamespace.bar.beta", + "nestednamespace.bar.beta.orange", +} + @pytest.mark.parametrize( - "package_name", - ("nestednamespace", "nestednamespace.foo", "nestednamespace.foo.alpha"), + "package_name, expected", + [ + # TODO: include the namespace packages as modules too. + ( + "nestednamespace", + FOO_ALPHA_BLUE_MODULES | FOO_ALPHA_GREEN_MODULES | BAR_BETA_MODULES, + ), + ( + "nestednamespace.foo", + FOO_ALPHA_BLUE_MODULES | FOO_ALPHA_GREEN_MODULES, + ), + ("nestednamespace.foo.alpha", FOO_ALPHA_BLUE_MODULES | FOO_ALPHA_GREEN_MODULES), + ], ) -def test_build_graph_for_nested_namespace(package_name): - with pytest.raises(exceptions.NamespacePackageEncountered): - build_graph(package_name, cache_dir=None) +def test_build_graph_for_nested_namespace(package_name, expected): + graph = build_graph(package_name, cache_dir=None) + + assert graph.modules == expected + assert graph.count_imports() @pytest.mark.parametrize( diff --git a/tests/unit/application/test_usecases.py b/tests/unit/application/test_usecases.py index 3922ce35..70821f6c 100644 --- a/tests/unit/application/test_usecases.py +++ b/tests/unit/application/test_usecases.py @@ -163,3 +163,48 @@ class FakePackageFinder(BaseFakePackageFinder): "mypackage.foo.one", } assert expected_modules == graph.modules + + def test_namespace_package_passed_as_root(self): + file_system = FakeFileSystem( + contents=""" + /path/to/mypackage/ + foo/ + __init__.py + one.py + two/ + __init__.py + green.py + blue.py + bar/ + __init__.py + three.py + /different-path/to/mypackage/ + foobar/ + __init__.py + four.py + """, + ) + + class FakePackageFinder(BaseFakePackageFinder): + directory_map = { + "mypackage": { + "/path/to/mypackage", + "/different-path/to/mypackage", + } + } + + with override_settings(FILE_SYSTEM=file_system, PACKAGE_FINDER=FakePackageFinder()): + graph = usecases.build_graph("mypackage") + + assert graph.modules == { + # TODO "mypackage" should also be included. + "mypackage.foo", + "mypackage.foo.one", + "mypackage.foo.two", + "mypackage.foo.two.green", + "mypackage.foo.two.blue", + "mypackage.bar", + "mypackage.bar.three", + "mypackage.foobar", + "mypackage.foobar.four", + } From 3f11a666286d9ca91db59ed3c0e0c311a5d35462 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Fri, 5 Dec 2025 09:19:59 +0000 Subject: [PATCH 15/21] Don't include directories that have no Python files within them --- src/grimp/adaptors/modulefinder.py | 20 +++++++++++++++++--- tests/unit/adaptors/test_modulefinder.py | 5 +++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/grimp/adaptors/modulefinder.py b/src/grimp/adaptors/modulefinder.py index 22f1a256..536731f5 100644 --- a/src/grimp/adaptors/modulefinder.py +++ b/src/grimp/adaptors/modulefinder.py @@ -53,8 +53,7 @@ def _get_python_files_and_namespace_dirs_inside_package( 2. Set of namespace directories encountered. """ python_files: list[str] = [] - namespace_dirs: set[str] = set() - + candidate_namespace_dirs: list[str] = [] portion_dirs: set[str] = set() for dirpath, dirs, files in self.file_system.walk(directory): @@ -71,7 +70,9 @@ def _get_python_files_and_namespace_dirs_inside_package( # This directory is a portion (i.e. it has a top-level __init__.py). portion_dirs.add(dirpath) else: - namespace_dirs.add(dirpath) + # We don't yet know whether this is a namespace dir. It'll only be one if we find + # a Python file somewhere within it. + candidate_namespace_dirs.append(dirpath) # Don't include hidden directories. dirs_to_remove = [d for d in dirs if self._should_ignore_dir(d)] @@ -82,6 +83,7 @@ def _get_python_files_and_namespace_dirs_inside_package( if self._is_python_file(filename, dirpath): python_files.append(self.file_system.join(dirpath, filename)) + namespace_dirs = self._determine_namespace_dirs(candidate_namespace_dirs, python_files) return python_files, namespace_dirs def _is_in_portion(self, directory: str, portions: Set[str]) -> bool: @@ -92,6 +94,18 @@ def _should_ignore_dir(self, directory: str) -> bool: # Skip adding directories that are hidden. return directory.startswith(".") + def _determine_namespace_dirs( + self, candidates: Iterable[str], python_files: Iterable[str] + ) -> set[str]: + namespace_dirs: set[str] = set() + for candidate in candidates: + candidate_with_trailing_sep = candidate + self.file_system.sep + for python_file in python_files: + if python_file.startswith(candidate_with_trailing_sep): + namespace_dirs.add(candidate) + break + return namespace_dirs + def _is_python_file(self, filename: str, dirpath: str) -> bool: """ Given a filename, return whether it's a Python file. diff --git a/tests/unit/adaptors/test_modulefinder.py b/tests/unit/adaptors/test_modulefinder.py index 97d3edb8..07354094 100644 --- a/tests/unit/adaptors/test_modulefinder.py +++ b/tests/unit/adaptors/test_modulefinder.py @@ -128,6 +128,11 @@ def test_namespaced_packages(package_name: str, package_directory: str, expected five/ __init__.py beta.py + non_python_directory/ + six/ + README.txt + + """ ) From 846b79610bb17e3ccb93e40dce83262287c4c403 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Fri, 5 Dec 2025 09:05:23 +0000 Subject: [PATCH 16/21] Include namespaces in graph --- src/grimp/application/usecases.py | 7 +++++- tests/functional/test_namespace_packages.py | 25 ++++++++++++++++----- tests/unit/application/test_usecases.py | 2 +- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/grimp/application/usecases.py b/src/grimp/application/usecases.py index 8341a449..68174259 100644 --- a/src/grimp/application/usecases.py +++ b/src/grimp/application/usecases.py @@ -3,7 +3,7 @@ """ from typing import cast - +import itertools from collections.abc import Sequence, Iterable from .scanning import scan_imports @@ -158,6 +158,11 @@ def _assemble_graph( ) -> ImportGraph: graph: ImportGraph = settings.IMPORT_GRAPH_CLASS() + for namespace_package in itertools.chain.from_iterable( + found_package.namespace_packages for found_package in found_packages + ): + graph.add_module(namespace_package) + package_modules = {Module(found_package.name) for found_package in found_packages} for module, direct_imports in imports_by_module.items(): diff --git a/tests/functional/test_namespace_packages.py b/tests/functional/test_namespace_packages.py index 697fc6cc..0b3dd1eb 100644 --- a/tests/functional/test_namespace_packages.py +++ b/tests/functional/test_namespace_packages.py @@ -25,7 +25,7 @@ def test_build_graph_for_namespace(): graph = build_graph("mynamespace", cache_dir=None) - assert graph.modules == YELLOW_MODULES | GREEN_MODULES | BLUE_MODULES + assert graph.modules == {"mynamespace"} | YELLOW_MODULES | GREEN_MODULES | BLUE_MODULES assert graph.count_imports() @@ -120,16 +120,31 @@ def test_import_between_namespace_children(): @pytest.mark.parametrize( "package_name, expected", [ - # TODO: include the namespace packages as modules too. ( "nestednamespace", - FOO_ALPHA_BLUE_MODULES | FOO_ALPHA_GREEN_MODULES | BAR_BETA_MODULES, + { + "nestednamespace", + "nestednamespace.foo", + "nestednamespace.foo.alpha", + "nestednamespace.bar", + } + | FOO_ALPHA_BLUE_MODULES + | FOO_ALPHA_GREEN_MODULES + | BAR_BETA_MODULES, ), ( "nestednamespace.foo", - FOO_ALPHA_BLUE_MODULES | FOO_ALPHA_GREEN_MODULES, + { + "nestednamespace.foo", + "nestednamespace.foo.alpha", + } + | FOO_ALPHA_BLUE_MODULES + | FOO_ALPHA_GREEN_MODULES, + ), + ( + "nestednamespace.foo.alpha", + {"nestednamespace.foo.alpha"} | FOO_ALPHA_BLUE_MODULES | FOO_ALPHA_GREEN_MODULES, ), - ("nestednamespace.foo.alpha", FOO_ALPHA_BLUE_MODULES | FOO_ALPHA_GREEN_MODULES), ], ) def test_build_graph_for_nested_namespace(package_name, expected): diff --git a/tests/unit/application/test_usecases.py b/tests/unit/application/test_usecases.py index 70821f6c..d8e2bc4c 100644 --- a/tests/unit/application/test_usecases.py +++ b/tests/unit/application/test_usecases.py @@ -197,7 +197,7 @@ class FakePackageFinder(BaseFakePackageFinder): graph = usecases.build_graph("mypackage") assert graph.modules == { - # TODO "mypackage" should also be included. + "mypackage", "mypackage.foo", "mypackage.foo.one", "mypackage.foo.two", From 60fc3a243fa01358dac75794b968aed914396820 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Mon, 8 Dec 2025 17:43:34 +0000 Subject: [PATCH 17/21] Update changelog --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fdbea30f..42fca449 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,7 @@ latest * Drop support for Python 3.9. * Bugfix: don't treat t-strings as syntax errors. https://github.com/python-grimp/grimp/issues/268 +* Support building graph from namespace packages, not just their portions. 3.13 (2025-10-29) ----------------- From 3a119883577c8b6190f3c1f44aa68434a78c8412 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Tue, 9 Dec 2025 07:45:04 +0000 Subject: [PATCH 18/21] Expand test to include building graph from root namespace --- tests/functional/test_namespace_packages.py | 22 ++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/functional/test_namespace_packages.py b/tests/functional/test_namespace_packages.py index 0b3dd1eb..ac47d2cb 100644 --- a/tests/functional/test_namespace_packages.py +++ b/tests/functional/test_namespace_packages.py @@ -197,13 +197,21 @@ def test_import_within_nested_namespace_child(): ) -def test_import_between_nested_namespace_children(): - graph = build_graph( - "nestednamespace.foo.alpha.blue", - "nestednamespace.foo.alpha.green", - "nestednamespace.bar.beta", - cache_dir=None, - ) +@pytest.mark.parametrize( + "root_packages", + ( + # As namespace root. + ("nestednamespace",), + # As portions. + ( + "nestednamespace.foo.alpha.blue", + "nestednamespace.foo.alpha.green", + "nestednamespace.bar.beta", + ), + ), +) +def test_import_between_nested_namespace_children(root_packages): + graph = build_graph(*root_packages, cache_dir=None) assert graph.direct_import_exists( importer="nestednamespace.foo.alpha.green.one", From a725e1908a8cd3d9a50082c036a0ee3079ef8c7b Mon Sep 17 00:00:00 2001 From: David Seddon Date: Tue, 9 Dec 2025 08:32:49 +0000 Subject: [PATCH 19/21] Don't drill down into invalid identifier directories This stops us from discovering Python modules that aren't importable. --- src/grimp/adaptors/modulefinder.py | 5 +-- tests/unit/adaptors/test_modulefinder.py | 54 ++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/grimp/adaptors/modulefinder.py b/src/grimp/adaptors/modulefinder.py index 536731f5..c38dc057 100644 --- a/src/grimp/adaptors/modulefinder.py +++ b/src/grimp/adaptors/modulefinder.py @@ -74,7 +74,7 @@ def _get_python_files_and_namespace_dirs_inside_package( # a Python file somewhere within it. candidate_namespace_dirs.append(dirpath) - # Don't include hidden directories. + # Don't include directories that aren't valid identifiers. dirs_to_remove = [d for d in dirs if self._should_ignore_dir(d)] for d in dirs_to_remove: dirs.remove(d) @@ -91,8 +91,7 @@ def _is_in_portion(self, directory: str, portions: Set[str]) -> bool: def _should_ignore_dir(self, directory: str) -> bool: # TODO: make this configurable. - # Skip adding directories that are hidden. - return directory.startswith(".") + return not directory.isidentifier() def _determine_namespace_dirs( self, candidates: Iterable[str], python_files: Iterable[str] diff --git a/tests/unit/adaptors/test_modulefinder.py b/tests/unit/adaptors/test_modulefinder.py index 07354094..8cf2e3fc 100644 --- a/tests/unit/adaptors/test_modulefinder.py +++ b/tests/unit/adaptors/test_modulefinder.py @@ -181,6 +181,60 @@ def test_ignores_orphaned_python_files(): ) +def test_ignores_invalid_identifier_directories(): + # Python files in directories that don't contain an __init__.py should not be discovered. + module_finder = ModuleFinder() + + file_system = FakeFileSystem( + contents=""" + /path/to/namespacepackage/ + foo/ + valid_underscore_name/ + __init__.py + mod.py + invalid-hyphenated-name/ + __init__.py + mod.py + 1nvalid-number-prefixed/ + __init__.py + mod.py + valid_non_àscii/ + __init__.py + mod.py + invalid_non_😃scii/ + __init__.py + mod.py + bar/ + __init__.py + mod.py + """ + ) + + result = module_finder.find_package( + package_name="namespacepackage", + package_directory="/path/to/namespacepackage", + file_system=file_system, + ) + + module_files = { + ModuleFile(module=Module(name), mtime=DEFAULT_MTIME) + for name in { + "namespacepackage.foo.valid_underscore_name", + "namespacepackage.foo.valid_underscore_name.mod", + "namespacepackage.foo.valid_non_àscii", + "namespacepackage.foo.valid_non_àscii.mod", + "namespacepackage.bar", + "namespacepackage.bar.mod", + } + } + assert result == FoundPackage( + name="namespacepackage", + directory="/path/to/namespacepackage", + module_files=module_files, + namespace_packages=frozenset({"namespacepackage", "namespacepackage.foo"}), + ) + + @pytest.mark.parametrize( "extension, should_warn", ( From 685f911b21c28cbaff29d3561842ee61cbc79ed5 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Wed, 10 Dec 2025 13:55:19 +0000 Subject: [PATCH 20/21] Include imports of namespace packages Prior to this, an import of a namespace package would be included if include_external_packages=True but not if it was False. Although in practice it's unlikely that a module would import a namespace package, this makes the behaviour consistent. --- rust/src/import_scanning.rs | 3 +++ rust/src/module_finding.rs | 21 +++++++++++++++++++-- tests/unit/application/test_usecases.py | 9 ++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/rust/src/import_scanning.rs b/rust/src/import_scanning.rs index dee2a420..a9e94786 100644 --- a/rust/src/import_scanning.rs +++ b/rust/src/import_scanning.rs @@ -58,6 +58,9 @@ fn get_modules_from_found_packages(found_packages: &HashSet) -> Ha for module_file in &package.module_files { modules.insert(module_file.module.clone()); } + for namespace_module in &package.namespace_packages { + modules.insert(namespace_module.clone()); + } } modules } diff --git a/rust/src/module_finding.rs b/rust/src/module_finding.rs index 4034b265..be01ed50 100644 --- a/rust/src/module_finding.rs +++ b/rust/src/module_finding.rs @@ -27,6 +27,7 @@ pub struct FoundPackage { pub directory: String, // BTreeSet rather than HashSet is necessary to make FoundPackage hashable. pub module_files: BTreeSet, + pub namespace_packages: BTreeSet, } /// Implements conversion from a Python 'FoundPackage' object to the Rust 'FoundPackage' struct. @@ -41,20 +42,36 @@ impl<'py> FromPyObject<'py> for FoundPackage { // Access the 'module_files' attribute. let module_files_py = ob.getattr("module_files")?; // Downcast the PyAny object to a PyFrozenSet, as Python 'FrozenSet' maps to 'PyFrozenSet'. - let py_frozen_set = module_files_py.downcast::()?; + let module_files_frozenset = module_files_py.downcast::()?; let mut module_files = BTreeSet::new(); // Iterate over the Python frozenset. - for py_module_file_any in py_frozen_set.iter() { + for py_module_file_any in module_files_frozenset.iter() { // Extract each element (PyAny) into a Rust 'ModuleFile'. let module_file: ModuleFile = py_module_file_any.extract()?; module_files.insert(module_file); } + // Access the 'namespace_packages' attribute. + let namespace_packages_py = ob.getattr("namespace_packages")?; + // Downcast the PyAny object to a PyFrozenSet, as Python 'FrozenSet' maps to 'PyFrozenSet'. + let namespace_packages_frozenset = namespace_packages_py.downcast::()?; + + let mut namespace_packages = BTreeSet::new(); + // Iterate over the Python frozenset. + for py_namespace_any in namespace_packages_frozenset.iter() { + // Extract each element (PyAny) into a Rust 'ModuleFile'. + let namespace_package: String = py_namespace_any.extract()?; + namespace_packages.insert(Module { + name: namespace_package, + }); + } + Ok(FoundPackage { name, directory, module_files, + namespace_packages, }) } } diff --git a/tests/unit/application/test_usecases.py b/tests/unit/application/test_usecases.py index d8e2bc4c..932fb4d9 100644 --- a/tests/unit/application/test_usecases.py +++ b/tests/unit/application/test_usecases.py @@ -183,6 +183,9 @@ def test_namespace_package_passed_as_root(self): __init__.py four.py """, + content_map={ + "/path/to/mypackage/foo/one.py": "import mypackage\nfrom . import two", + }, ) class FakePackageFinder(BaseFakePackageFinder): @@ -190,7 +193,7 @@ class FakePackageFinder(BaseFakePackageFinder): "mypackage": { "/path/to/mypackage", "/different-path/to/mypackage", - } + }, } with override_settings(FILE_SYSTEM=file_system, PACKAGE_FINDER=FakePackageFinder()): @@ -208,3 +211,7 @@ class FakePackageFinder(BaseFakePackageFinder): "mypackage.foobar", "mypackage.foobar.four", } + assert graph.find_modules_directly_imported_by("mypackage.foo.one") == { + "mypackage", + "mypackage.foo.two", + } From c76a1d48cce385a01b2eeaa8f9e1c690f108c815 Mon Sep 17 00:00:00 2001 From: David Seddon Date: Wed, 10 Dec 2025 16:08:59 +0000 Subject: [PATCH 21/21] Add docs for better namespace support --- README.rst | 31 ++++++++++++++++++++++++------- docs/usage.rst | 9 +++++---- src/grimp/application/usecases.py | 2 +- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/README.rst b/README.rst index f3f78a91..dc51488c 100644 --- a/README.rst +++ b/README.rst @@ -114,20 +114,37 @@ You may analyse multiple root packages. To do this, pass each package name as a Namespace packages ------------------ -Graphs can also be built from `portions`_ of `namespace packages`_. To do this, provide the portion name, rather than the namespace name:: - - >>> graph = grimp.build_graph('somenamespace.foo') +Graphs can be built either from `namespace packages`_ or from their `portions`_. What's a namespace package? ########################### -Namespace packages are a Python feature allows subpackages to be distributed independently, while still importable under a shared namespace. This is, for example, used by `the Python client for Google's Cloud Logging API`_. When installed, it is importable in Python as ``google.cloud.logging``. The parent packages ``google`` and ``google.cloud`` are both namespace packages, while ``google.cloud.logging`` is known as the 'portion'. Other portions in the same namespace can be installed separately, for example ``google.cloud.secretmanager``. +Namespace packages are a Python feature allows subpackages to be distributed independently, while +still importable under a shared namespace. + +This is used by +`the Python client for Google's Cloud Logging API`_, for example. When installed, it is importable +in Python as ``google.cloud.logging``. The parent packages ``google`` and ``google.cloud`` are both namespace +packages, while ``google.cloud.logging`` is known as the 'portion'. Other portions in the same +namespace can be installed separately, for example ``google.cloud.secretmanager``. + +Examples:: + + # In this one, the portion is supplied. Neither "google" nor "google.cloud" + # will appear in the graph. + >>> graph = grimp.build_graph("google.cloud.logging") + + # In this one, a namespace is supplied. + # Neither "google" nor "google.cloud" will appear in the graph, + # as will other installed packages under the "google" namespace such + # as "google.auth". + >>> graph = grimp.build_graph("google") -Grimp expects the package name passed to ``build_graph`` to be a portion, rather than a namespace package. So in the case of the example above, the graph should be built like so:: + # This one supplies a subnamespace of "google" - it will include + # "google.cloud.logging" and "google.cloud.secretmanager" but not "google.auth". + >>> graph = grimp.build_graph("google.cloud") - >>> graph = grimp.build_graph('google.cloud.logging') -If, instead, a namespace package is passed (e.g. ``grimp.build_graph('google.cloud')``), Grimp will raise ``NamespacePackageEncountered``. .. _portions: https://docs.python.org/3/glossary.html#term-portion .. _namespace packages: https://docs.python.org/3/glossary.html#term-namespace-package diff --git a/docs/usage.rst b/docs/usage.rst index 1551b8cc..169ef18c 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -57,19 +57,20 @@ Building the graph Build and return an ImportGraph for the supplied package or packages. :param str package_name: The name of an importable package, for example ``'mypackage'``. For regular packages, this - must be the top level package (i.e. one with no dots in its name). However, in the special case of - `namespace packages`_, the name of the *portion* should be supplied, for example ``'mynamespace.foo'``. + must be the top level package (i.e. one with no dots in its name). In the special case of + `namespace packages`_, the name of the *portion* may be supplied instead, for example ``'mynamespace.foo'``. + If the portion is supplied, its ancestor packages will not be included in the graph. :param tuple[str, ...] additional_package_names: Tuple of any additional package names. These can be supplied as positional arguments, as in the example above. :param bool, optional include_external_packages: Whether to include external packages in the import graph. If this is ``True``, any other top level packages (including packages in the standard library) that are imported by this package will be included in the graph as squashed modules (see `Terminology`_ above). - The behaviour is more complex if one of the internal packages is a `namespace portion`_. + The behaviour is more complex if one of the specified packages is a `namespace portion`_. In this case, the squashed module will have the shallowest name that doesn't clash with any internal modules. For example, in a graph with internal packages ``namespace.foo`` and ``namespace.bar.one.green``, ``namespace.bar.one.orange.alpha`` would be added to the graph as ``namespace.bar.one.orange``. However, in a graph - with only ``namespace.foo`` as an internal package, the same external module would be added as + with only ``namespace.foo`` passed, the same external module would be added as ``namespace.bar``. *Note: external packages are only analysed as modules that are imported; any imports they make themselves will diff --git a/src/grimp/application/usecases.py b/src/grimp/application/usecases.py index 68174259..4765b7ed 100644 --- a/src/grimp/application/usecases.py +++ b/src/grimp/application/usecases.py @@ -32,7 +32,7 @@ def build_graph( Args: - package_name: the name of the top level package for which to build the graph. - - additional_package_names: tuple of the + - additional_package_names: tuple of additional packages to build the graph from. - include_external_packages: whether to include any external packages in the graph. - exclude_type_checking_imports: whether to exclude imports made in type checking guards. - cache_dir: The directory to use for caching the graph.