From 5cd7df709b858a8a62c5006b2ddb617d18b6213a Mon Sep 17 00:00:00 2001 From: Peter Byfield Date: Sat, 8 Feb 2025 17:50:42 +0100 Subject: [PATCH 1/2] Add as_packages parameter to find_shortest_chain The implementation of the pathfinding already supports this, so let's add this to the public API. --- CHANGELOG.rst | 5 +++++ docs/usage.rst | 6 +++++- rust/src/lib.rs | 5 +++-- src/grimp/adaptors/graph.py | 6 ++++-- src/grimp/application/ports/graph.py | 10 +++++++++- tests/unit/adaptors/graph/test_chains.py | 19 +++++++++++++++++++ 6 files changed, 45 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 809d21d0..f1b2f40c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog ========= +Unreleased +---------- + +* Added `as_packages` option to the `find_shortest_chain` method. + 3.6 (2025-02-07) ---------------- diff --git a/docs/usage.rst b/docs/usage.rst index 55d2698c..77927d48 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -216,11 +216,15 @@ Methods for analysing import chains :return: All the modules that are imported (even indirectly) by the supplied module. :rtype: A set of strings. -.. py:function:: ImportGraph.find_shortest_chain(importer, imported) +.. py:function:: ImportGraph.find_shortest_chain(importer, imported, as_packages=False) :param str importer: The module at the start of a potential chain of imports between ``importer`` and ``imported`` (i.e. the module that potentially imports ``imported``, even indirectly). :param str imported: The module at the end of the potential chain of imports. + :param bool as_packages: Whether to treat the supplied modules as individual modules, + or as packages (including any descendants, if there are any). If + treating them as packages, all descendants of ``importer`` and + ``imported`` will be checked too. :return: The shortest chain of imports between the supplied modules, or None if no chain exists. :rtype: A tuple of strings, ordered from importer to imported modules, or None. diff --git a/rust/src/lib.rs b/rust/src/lib.rs index d293f852..f65c2521 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -296,17 +296,18 @@ impl GraphWrapper { .collect()) } - // TODO(peter) Add `as_packages` argument here? The implementation already supports it! + #[pyo3(signature = (importer, imported, as_packages=false))] pub fn find_shortest_chain( &self, importer: &str, imported: &str, + as_packages: bool, ) -> PyResult>> { let importer = self.get_visible_module_by_name(importer)?.token(); let imported = self.get_visible_module_by_name(imported)?.token(); Ok(self ._graph - .find_shortest_chain(importer, imported, false)? + .find_shortest_chain(importer, imported, as_packages)? .map(|chain| { chain .iter() diff --git a/src/grimp/adaptors/graph.py b/src/grimp/adaptors/graph.py index f4abe0ca..4730ef45 100644 --- a/src/grimp/adaptors/graph.py +++ b/src/grimp/adaptors/graph.py @@ -108,12 +108,14 @@ def find_downstream_modules(self, module: str, as_package: bool = False) -> Set[ def find_upstream_modules(self, module: str, as_package: bool = False) -> Set[str]: return self._rustgraph.find_upstream_modules(module, as_package) - def find_shortest_chain(self, importer: str, imported: str) -> tuple[str, ...] | None: + def find_shortest_chain( + self, importer: str, imported: str, as_packages: bool = False + ) -> tuple[str, ...] | None: for module in (importer, imported): if not self._rustgraph.contains_module(module): raise ValueError(f"Module {module} is not present in the graph.") - chain = self._rustgraph.find_shortest_chain(importer, imported) + chain = self._rustgraph.find_shortest_chain(importer, imported, as_packages) return tuple(chain) if chain else None def find_shortest_chains( diff --git a/src/grimp/application/ports/graph.py b/src/grimp/application/ports/graph.py index cf9ec1bb..acf39a84 100644 --- a/src/grimp/application/ports/graph.py +++ b/src/grimp/application/ports/graph.py @@ -220,11 +220,19 @@ def find_upstream_modules(self, module: str, as_package: bool = False) -> Set[st raise NotImplementedError @abc.abstractmethod - def find_shortest_chain(self, importer: str, imported: str) -> tuple[str, ...] | None: + def find_shortest_chain( + self, importer: str, imported: str, as_packages: bool = False + ) -> tuple[str, ...] | None: """ Attempt to find the shortest chain of imports between two modules, in the direction of importer to imported. + Optional args: + as_packages: Whether to treat the supplied modules as individual modules, + or as packages (including any descendants, if there are any). If + treating them as subpackages, all descendants of the supplied modules + will be checked too. + Returns: Tuple of module names, from importer to imported, or None if no chain exists. """ diff --git a/tests/unit/adaptors/graph/test_chains.py b/tests/unit/adaptors/graph/test_chains.py index 98965fd4..a6d983aa 100644 --- a/tests/unit/adaptors/graph/test_chains.py +++ b/tests/unit/adaptors/graph/test_chains.py @@ -168,6 +168,25 @@ def test_demonstrate_nondeterminism_of_equal_chains(self): other_chain = (source, d, e, f, destination) assert (result == one_chain) or (result == other_chain) + @pytest.mark.parametrize( + "as_packages, expected_result", + ( + (False, None), + (True, ("green.foo", "blue.bar")), + ), + ) + def test_as_packages(self, as_packages: bool, expected_result: Set[Tuple]): + graph = ImportGraph() + graph.add_module("green") + graph.add_module("blue") + graph.add_import(importer="green.foo", imported="blue.bar") + + result = graph.find_shortest_chain( + importer="green", imported="blue", as_packages=as_packages + ) + + assert result == expected_result + class TestFindShortestChains: @pytest.mark.parametrize( From 633f81dd53f3b894342dfc261f24e673d4d8c169 Mon Sep 17 00:00:00 2001 From: Peter Byfield Date: Mon, 10 Feb 2025 21:43:46 +0100 Subject: [PATCH 2/2] Add as_packages to find_shortest_chains signature in usage doc --- docs/usage.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage.rst b/docs/usage.rst index 77927d48..ba336efa 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -228,7 +228,7 @@ Methods for analysing import chains :return: The shortest chain of imports between the supplied modules, or None if no chain exists. :rtype: A tuple of strings, ordered from importer to imported modules, or None. -.. py:function:: ImportGraph.find_shortest_chains(importer, imported) +.. py:function:: ImportGraph.find_shortest_chains(importer, imported, as_packages=True) :param str importer: A module or subpackage within the graph. :param str imported: Another module or subpackage within the graph.