From 20bd0640e16915c62f356a1b94594ec08095c678 Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Mon, 30 Mar 2026 22:10:26 -0700 Subject: [PATCH] [Library] improve handling of dangling refs --- masque/file/utils.py | 3 +- masque/library.py | 127 ++++++++++++++++++++++++++++++------ masque/test/test_library.py | 75 +++++++++++++++++++++ 3 files changed, 183 insertions(+), 22 deletions(-) diff --git a/masque/file/utils.py b/masque/file/utils.py index 33f68d4..25bc61d 100644 --- a/masque/file/utils.py +++ b/masque/file/utils.py @@ -75,7 +75,8 @@ def preflight( raise PatternError('Non-numeric layers found:' + pformat(named_layers)) if prune_empty_patterns: - pruned = lib.prune_empty() + prune_dangling = 'error' if allow_dangling_refs is False else 'ignore' + pruned = lib.prune_empty(dangling=prune_dangling) if pruned: logger.info(f'Preflight pruned {len(pruned)} empty patterns') logger.debug('Pruned: ' + pformat(pruned)) diff --git a/masque/library.py b/masque/library.py index afd25c6..cb50139 100644 --- a/masque/library.py +++ b/masque/library.py @@ -59,6 +59,9 @@ TreeView: TypeAlias = Mapping[str, 'Pattern'] Tree: TypeAlias = MutableMapping[str, 'Pattern'] """ A mutable name-to-`Pattern` mapping which is expected to have only one top-level cell """ +dangling_mode_t: TypeAlias = Literal['error', 'ignore', 'include'] +""" How helpers should handle refs whose targets are not present in the library. """ + SINGLE_USE_PREFIX = '_' """ @@ -418,6 +421,21 @@ class ILibraryView(Mapping[str, 'Pattern'], metaclass=ABCMeta): """ return self[self.top()] + @staticmethod + def _dangling_refs_error(dangling: set[str], context: str) -> LibraryError: + dangling_list = sorted(dangling) + return LibraryError(f'Dangling refs found while {context}: ' + pformat(dangling_list)) + + def _raw_child_graph(self) -> tuple[dict[str, set[str]], set[str]]: + existing = set(self.keys()) + graph: dict[str, set[str]] = {} + dangling: set[str] = set() + for name, pat in self.items(): + children = {child for child, refs in pat.refs.items() if child is not None and refs} + graph[name] = children + dangling |= children - existing + return graph, dangling + def dfs( self, pattern: 'Pattern', @@ -523,46 +541,88 @@ class ILibraryView(Mapping[str, 'Pattern'], metaclass=ABCMeta): return self - def child_graph(self) -> dict[str, set[str | None]]: + def child_graph( + self, + dangling: dangling_mode_t = 'error', + ) -> dict[str, set[str]]: """ Return a mapping from pattern name to a set of all child patterns (patterns it references). + Only non-empty ref lists with non-`None` targets are treated as graph edges. + + Args: + dangling: How refs to missing targets are handled. `'error'` raises, + `'ignore'` drops those edges, and `'include'` exposes them as + synthetic leaf nodes. + Returns: Mapping from pattern name to a set of all pattern names it references. """ - graph = {name: set(pat.refs.keys()) for name, pat in self.items()} + graph, dangling_refs = self._raw_child_graph() + if dangling == 'error': + if dangling_refs: + raise self._dangling_refs_error(dangling_refs, 'building child graph') + return graph + if dangling == 'ignore': + existing = set(graph) + return {name: {child for child in children if child in existing} for name, children in graph.items()} + + for target in dangling_refs: + graph.setdefault(target, set()) return graph - def parent_graph(self) -> dict[str, set[str]]: + def parent_graph( + self, + dangling: dangling_mode_t = 'error', + ) -> dict[str, set[str]]: """ Return a mapping from pattern name to a set of all parent patterns (patterns which reference it). + Args: + dangling: How refs to missing targets are handled. `'error'` raises, + `'ignore'` drops those targets, and `'include'` adds them as + synthetic keys whose values are their existing parents. + Returns: Mapping from pattern name to a set of all patterns which reference it. """ - igraph: dict[str, set[str]] = {name: set() for name in self} - for name, pat in self.items(): - for child, reflist in pat.refs.items(): - if reflist and child is not None: - igraph[child].add(name) + child_graph, dangling_refs = self._raw_child_graph() + if dangling == 'error' and dangling_refs: + raise self._dangling_refs_error(dangling_refs, 'building parent graph') + + existing = set(child_graph) + igraph: dict[str, set[str]] = {name: set() for name in existing} + for parent, children in child_graph.items(): + for child in children: + if child in existing: + igraph[child].add(parent) + elif dangling == 'include': + igraph.setdefault(child, set()).add(parent) return igraph - def child_order(self) -> list[str]: + def child_order( + self, + dangling: dangling_mode_t = 'error', + ) -> list[str]: """ - Return a topologically sorted list of all contained pattern names. + Return a topologically sorted list of graph node names. Child (referenced) patterns will appear before their parents. + Args: + dangling: Passed to `child_graph()`. + Return: Topologically sorted list of pattern names. """ - return cast('list[str]', list(TopologicalSorter(self.child_graph()).static_order())) + return cast('list[str]', list(TopologicalSorter(self.child_graph(dangling=dangling)).static_order())) def find_refs_local( self, name: str, parent_graph: dict[str, set[str]] | None = None, + dangling: dangling_mode_t = 'error', ) -> dict[str, list[NDArray[numpy.float64]]]: """ Find the location and orientation of all refs pointing to `name`. @@ -575,6 +635,8 @@ class ILibraryView(Mapping[str, 'Pattern'], metaclass=ABCMeta): The provided graph may be for a superset of `self` (i.e. it may contain additional patterns which are not present in self; they will be ignored). + dangling: How refs to missing targets are handled if `parent_graph` + is not provided. `'include'` also allows querying missing names. Returns: Mapping of {parent_name: transform_list}, where transform_list @@ -583,8 +645,18 @@ class ILibraryView(Mapping[str, 'Pattern'], metaclass=ABCMeta): """ instances = defaultdict(list) if parent_graph is None: - parent_graph = self.parent_graph() - for parent in parent_graph[name]: + graph_mode = 'ignore' if dangling == 'ignore' else 'include' + parent_graph = self.parent_graph(dangling=graph_mode) + + if name not in self: + if name not in parent_graph: + return instances + if dangling == 'error': + raise self._dangling_refs_error({name}, f'finding local refs for {name!r}') + if dangling == 'ignore': + return instances + + for parent in parent_graph.get(name, set()): if parent not in self: # parent_graph may be a for a superset of self continue for ref in self[parent].refs[name]: @@ -597,6 +669,7 @@ class ILibraryView(Mapping[str, 'Pattern'], metaclass=ABCMeta): name: str, order: list[str] | None = None, parent_graph: dict[str, set[str]] | None = None, + dangling: dangling_mode_t = 'error', ) -> dict[tuple[str, ...], NDArray[numpy.float64]]: """ Find the absolute (top-level) location and orientation of all refs (including @@ -613,18 +686,28 @@ class ILibraryView(Mapping[str, 'Pattern'], metaclass=ABCMeta): The provided graph may be for a superset of `self` (i.e. it may contain additional patterns which are not present in self; they will be ignored). + dangling: How refs to missing targets are handled if `order` or + `parent_graph` are not provided. `'include'` also allows + querying missing names. Returns: Mapping of `{hierarchy: transform_list}`, where `hierarchy` is a tuple of the form `(toplevel_pattern, lvl1_pattern, ..., name)` and `transform_list` is an Nx4 ndarray with rows `(x_offset, y_offset, rotation_ccw_rad, mirror_across_x)`. """ - if name not in self: - return {} + graph_mode = 'ignore' if dangling == 'ignore' else 'include' if order is None: - order = self.child_order() + order = self.child_order(dangling=graph_mode) if parent_graph is None: - parent_graph = self.parent_graph() + parent_graph = self.parent_graph(dangling=graph_mode) + + if name not in self: + if name not in parent_graph: + return {} + if dangling == 'error': + raise self._dangling_refs_error({name}, f'finding global refs for {name!r}') + if dangling == 'ignore': + return {} self_keys = set(self.keys()) @@ -633,16 +716,16 @@ class ILibraryView(Mapping[str, 'Pattern'], metaclass=ABCMeta): NDArray[numpy.float64] ]]] transforms = defaultdict(list) - for parent, vals in self.find_refs_local(name, parent_graph=parent_graph).items(): + for parent, vals in self.find_refs_local(name, parent_graph=parent_graph, dangling=dangling).items(): transforms[parent] = [((name,), numpy.concatenate(vals))] for next_name in order: if next_name not in transforms: continue - if not parent_graph[next_name] & self_keys: + if not parent_graph.get(next_name, set()) & self_keys: continue - outers = self.find_refs_local(next_name, parent_graph=parent_graph) + outers = self.find_refs_local(next_name, parent_graph=parent_graph, dangling=dangling) inners = transforms.pop(next_name) for parent, outer in outers.items(): for path, inner in inners: @@ -1119,17 +1202,19 @@ class ILibrary(ILibraryView, MutableMapping[str, 'Pattern'], metaclass=ABCMeta): def prune_empty( self, repeat: bool = True, + dangling: dangling_mode_t = 'error', ) -> set[str]: """ Delete any empty patterns (i.e. where `Pattern.is_empty` returns `True`). Args: repeat: Also recursively delete any patterns which only contain(ed) empty patterns. + dangling: Passed to `parent_graph()`. Returns: A set containing the names of all deleted patterns """ - parent_graph = self.parent_graph() + parent_graph = self.parent_graph(dangling=dangling) empty = {name for name, pat in self.items() if pat.is_empty()} trimmed = set() while empty: diff --git a/masque/test/test_library.py b/masque/test/test_library.py index 9eb705e..1f65595 100644 --- a/masque/test/test_library.py +++ b/masque/test/test_library.py @@ -1,10 +1,12 @@ import pytest from typing import cast, TYPE_CHECKING +from numpy.testing import assert_allclose from ..library import Library, LazyLibrary from ..pattern import Pattern from ..error import LibraryError, PatternError from ..ports import Port from ..repetition import Grid +from ..file.utils import preflight if TYPE_CHECKING: from ..shapes import Polygon @@ -41,6 +43,79 @@ def test_library_dangling() -> None: assert lib.dangling_refs() == {"missing"} +def test_library_dangling_graph_modes() -> None: + lib = Library() + lib["parent"] = Pattern() + lib["parent"].ref("missing") + + with pytest.raises(LibraryError, match="Dangling refs found"): + lib.child_graph() + with pytest.raises(LibraryError, match="Dangling refs found"): + lib.parent_graph() + with pytest.raises(LibraryError, match="Dangling refs found"): + lib.child_order() + + assert lib.child_graph(dangling="ignore") == {"parent": set()} + assert lib.parent_graph(dangling="ignore") == {"parent": set()} + assert lib.child_order(dangling="ignore") == ["parent"] + + assert lib.child_graph(dangling="include") == {"parent": {"missing"}, "missing": set()} + assert lib.parent_graph(dangling="include") == {"parent": set(), "missing": {"parent"}} + assert lib.child_order(dangling="include") == ["missing", "parent"] + + +def test_find_refs_with_dangling_modes() -> None: + lib = Library() + lib["target"] = Pattern() + + mid = Pattern() + mid.ref("target", offset=(2, 0)) + lib["mid"] = mid + + top = Pattern() + top.ref("mid", offset=(5, 0)) + top.ref("missing", offset=(9, 0)) + lib["top"] = top + + assert lib.find_refs_local("missing", dangling="ignore") == {} + assert lib.find_refs_global("missing", dangling="ignore") == {} + + local_missing = lib.find_refs_local("missing", dangling="include") + assert set(local_missing) == {"top"} + assert_allclose(local_missing["top"][0], [[9, 0, 0, 0, 1]]) + + global_missing = lib.find_refs_global("missing", dangling="include") + assert_allclose(global_missing[("top", "missing")], [[9, 0, 0, 0, 1]]) + + with pytest.raises(LibraryError, match="missing"): + lib.find_refs_local("missing") + with pytest.raises(LibraryError, match="missing"): + lib.find_refs_global("missing") + + global_target = lib.find_refs_global("target") + assert_allclose(global_target[("top", "mid", "target")], [[7, 0, 0, 0, 1]]) + + +def test_preflight_prune_empty_preserves_dangling_policy(caplog: pytest.LogCaptureFixture) -> None: + def make_lib() -> Library: + lib = Library() + lib["empty"] = Pattern() + lib["top"] = Pattern() + lib["top"].ref("missing") + return lib + + caplog.set_level("WARNING") + warned = preflight(make_lib(), allow_dangling_refs=None, prune_empty_patterns=True) + assert "empty" not in warned + assert any("Dangling refs found" in record.message for record in caplog.records) + + allowed = preflight(make_lib(), allow_dangling_refs=True, prune_empty_patterns=True) + assert "empty" not in allowed + + with pytest.raises(LibraryError, match="Dangling refs found"): + preflight(make_lib(), allow_dangling_refs=False, prune_empty_patterns=True) + + def test_library_flatten() -> None: lib = Library() child = Pattern()