diff --git a/masque/label.py b/masque/label.py index 3dbbc08..6a2a61f 100644 --- a/masque/label.py +++ b/masque/label.py @@ -65,6 +65,7 @@ class Label(PositionableImpl, RepeatableImpl, AnnotatableImpl, Bounded, Pivotabl memo = {} if memo is None else memo new = copy.copy(self) new._offset = self._offset.copy() + new._repetition = copy.deepcopy(self._repetition, memo) new._annotations = copy.deepcopy(self._annotations, memo) return new diff --git a/masque/pattern.py b/masque/pattern.py index ab5f55a..b670493 100644 --- a/masque/pattern.py +++ b/masque/pattern.py @@ -349,6 +349,16 @@ class Pattern(PortList, AnnotatableImpl, Mirrorable): Returns: self """ + annotation_conflicts: set[str] = set() + if other_pattern.annotations is not None and self.annotations is not None: + annotation_conflicts = set(self.annotations.keys()) & set(other_pattern.annotations.keys()) + if annotation_conflicts: + raise PatternError(f'Annotation keys overlap: {annotation_conflicts}') + + port_conflicts = set(self.ports.keys()) & set(other_pattern.ports.keys()) + if port_conflicts: + raise PatternError(f'Port names overlap: {port_conflicts}') + for target, rseq in other_pattern.refs.items(): self.refs[target].extend(rseq) for layer, sseq in other_pattern.shapes.items(): @@ -359,14 +369,7 @@ class Pattern(PortList, AnnotatableImpl, Mirrorable): if other_pattern.annotations is not None: if self.annotations is None: self.annotations = {} - annotation_conflicts = set(self.annotations.keys()) & set(other_pattern.annotations.keys()) - if annotation_conflicts: - raise PatternError(f'Annotation keys overlap: {annotation_conflicts}') self.annotations.update(other_pattern.annotations) - - port_conflicts = set(self.ports.keys()) & set(other_pattern.ports.keys()) - if port_conflicts: - raise PatternError(f'Port names overlap: {port_conflicts}') self.ports.update(other_pattern.ports) return self @@ -1380,6 +1383,15 @@ class Pattern(PortList, AnnotatableImpl, Mirrorable): if not skip_port_check: self.check_ports(other.ports.keys(), map_in=None, map_out=port_map) + if not skip_geometry: + if append: + if isinstance(other, Abstract): + raise PatternError('Must provide a full `Pattern` (not an `Abstract`) when appending!') + else: + if isinstance(other, Pattern): + raise PatternError('Must provide an `Abstract` (not a `Pattern`) when creating a reference. ' + 'Use `append=True` if you intended to append the full geometry.') + ports = {} for name, port in other.ports.items(): new_name = port_map.get(name, name) @@ -1401,8 +1413,6 @@ class Pattern(PortList, AnnotatableImpl, Mirrorable): return self if append: - if isinstance(other, Abstract): - raise PatternError('Must provide a full `Pattern` (not an `Abstract`) when appending!') other_copy = other.deepcopy() other_copy.ports.clear() if mirrored: @@ -1411,9 +1421,6 @@ class Pattern(PortList, AnnotatableImpl, Mirrorable): other_copy.translate_elements(offset) self.append(other_copy) else: - if isinstance(other, Pattern): - raise PatternError('Must provide an `Abstract` (not a `Pattern`) when creating a reference. ' - 'Use `append=True` if you intended to append the full geometry.') ref = Ref(mirrored=mirrored) ref.rotate_around(pivot, rotation) ref.translate(offset) @@ -1551,6 +1558,13 @@ class Pattern(PortList, AnnotatableImpl, Mirrorable): map_out = {out_port_name: next(iter(map_in.keys()))} self.check_ports(other.ports.keys(), map_in, map_out) + if not skip_geometry: + if append: + if isinstance(other, Abstract): + raise PatternError('Must provide a full `Pattern` (not an `Abstract`) when appending!') + elif isinstance(other, Pattern): + raise PatternError('Must provide an `Abstract` (not a `Pattern`) when creating a reference. ' + 'Use `append=True` if you intended to append the full geometry.') try: translation, rotation, pivot = self.find_transform( other, @@ -1584,10 +1598,6 @@ class Pattern(PortList, AnnotatableImpl, Mirrorable): self._log_port_removal(ki) map_out[vi] = None - if isinstance(other, Pattern) and not (append or skip_geometry): - raise PatternError('Must provide an `Abstract` (not a `Pattern`) when creating a reference. ' - 'Use `append=True` if you intended to append the full geometry.') - self.place( other, offset = translation, @@ -1659,9 +1669,13 @@ class Pattern(PortList, AnnotatableImpl, Mirrorable): else: raise PatternError(f'Unable to get ports from {type(source)}: {source}') - if port_map: + if port_map is not None: if isinstance(port_map, dict): missing_inkeys = set(port_map.keys()) - set(orig_ports.keys()) + port_targets = list(port_map.values()) + duplicate_targets = {vv for vv in port_targets if port_targets.count(vv) > 1} + if duplicate_targets: + raise PortError(f'Duplicate targets in `port_map`: {duplicate_targets}') mapped_ports = {port_map[k]: v for k, v in orig_ports.items() if k in port_map} else: port_set = set(port_map) diff --git a/masque/ports.py b/masque/ports.py index 3a67003..8d7b6a6 100644 --- a/masque/ports.py +++ b/masque/ports.py @@ -331,6 +331,10 @@ class PortList(metaclass=ABCMeta): missing = set(mapping) - set(self.ports) if missing: raise PortError(f'Ports to rename were not found: {missing}') + renamed_targets = [vv for vv in mapping.values() if vv is not None] + duplicate_targets = {vv for vv in renamed_targets if renamed_targets.count(vv) > 1} + if duplicate_targets: + raise PortError(f'Renamed ports would collide: {duplicate_targets}') for kk, vv in mapping.items(): if vv is None or vv != kk: @@ -409,6 +413,10 @@ class PortList(metaclass=ABCMeta): if missing_b: raise PortError(f'Connection destination ports were not found: {missing_b}') a_names, b_names = list(zip(*connections.items(), strict=True)) + used_names = list(chain(a_names, b_names)) + duplicate_names = {name for name in used_names if used_names.count(name) > 1} + if duplicate_names: + raise PortError(f'Each port may appear in at most one connection: {duplicate_names}') a_ports = [self.ports[pp] for pp in a_names] b_ports = [self.ports[pp] for pp in b_names] diff --git a/masque/shapes/arc.py b/masque/shapes/arc.py index ab2e47b..2adfe10 100644 --- a/masque/shapes/arc.py +++ b/masque/shapes/arc.py @@ -54,8 +54,8 @@ class Arc(PositionableImpl, Shape): val = numpy.array(val, dtype=float).flatten() if not val.size == 2: raise PatternError('Radii must have length 2') - if not val.min() >= 0: - raise PatternError('Radii must be non-negative') + if not val.min() > 0: + raise PatternError('Radii must be positive') self._radii = val @property @@ -64,8 +64,8 @@ class Arc(PositionableImpl, Shape): @radius_x.setter def radius_x(self, val: float) -> None: - if not val >= 0: - raise PatternError('Radius must be non-negative') + if not val > 0: + raise PatternError('Radius must be positive') self._radii[0] = val @property @@ -74,8 +74,8 @@ class Arc(PositionableImpl, Shape): @radius_y.setter def radius_y(self, val: float) -> None: - if not val >= 0: - raise PatternError('Radius must be non-negative') + if not val > 0: + raise PatternError('Radius must be positive') self._radii[1] = val # arc start/stop angle properties @@ -187,6 +187,7 @@ class Arc(PositionableImpl, Shape): new._offset = self._offset.copy() new._radii = self._radii.copy() new._angles = self._angles.copy() + new._repetition = copy.deepcopy(self._repetition, memo) new._annotations = copy.deepcopy(self._annotations) return new diff --git a/masque/shapes/circle.py b/masque/shapes/circle.py index a6f7af1..19f3f2b 100644 --- a/masque/shapes/circle.py +++ b/masque/shapes/circle.py @@ -68,6 +68,7 @@ class Circle(PositionableImpl, Shape): memo = {} if memo is None else memo new = copy.copy(self) new._offset = self._offset.copy() + new._repetition = copy.deepcopy(self._repetition, memo) new._annotations = copy.deepcopy(self._annotations) return new diff --git a/masque/shapes/ellipse.py b/masque/shapes/ellipse.py index 55ce9fd..38799da 100644 --- a/masque/shapes/ellipse.py +++ b/masque/shapes/ellipse.py @@ -42,7 +42,7 @@ class Ellipse(PositionableImpl, Shape): @radii.setter def radii(self, val: ArrayLike) -> None: - val = numpy.array(val).flatten() + val = numpy.array(val, dtype=float).flatten() if not val.size == 2: raise PatternError('Radii must have length 2') if not val.min() >= 0: @@ -117,6 +117,7 @@ class Ellipse(PositionableImpl, Shape): new = copy.copy(self) new._offset = self._offset.copy() new._radii = self._radii.copy() + new._repetition = copy.deepcopy(self._repetition, memo) new._annotations = copy.deepcopy(self._annotations) return new diff --git a/masque/shapes/path.py b/masque/shapes/path.py index 3aa6f07..7038309 100644 --- a/masque/shapes/path.py +++ b/masque/shapes/path.py @@ -235,6 +235,7 @@ class Path(Shape): new._vertices = self._vertices.copy() new._cap = copy.deepcopy(self._cap, memo) new._cap_extensions = copy.deepcopy(self._cap_extensions, memo) + new._repetition = copy.deepcopy(self._repetition, memo) new._annotations = copy.deepcopy(self._annotations) return new diff --git a/masque/shapes/poly_collection.py b/masque/shapes/poly_collection.py index 711acc4..cd233ba 100644 --- a/masque/shapes/poly_collection.py +++ b/masque/shapes/poly_collection.py @@ -124,6 +124,7 @@ class PolyCollection(Shape): new = copy.copy(self) new._vertex_lists = self._vertex_lists.copy() new._vertex_offsets = self._vertex_offsets.copy() + new._repetition = copy.deepcopy(self._repetition, memo) new._annotations = copy.deepcopy(self._annotations) return new diff --git a/masque/shapes/polygon.py b/masque/shapes/polygon.py index 34a784b..a743de2 100644 --- a/masque/shapes/polygon.py +++ b/masque/shapes/polygon.py @@ -135,6 +135,7 @@ class Polygon(Shape): memo = {} if memo is None else memo new = copy.copy(self) new._vertices = self._vertices.copy() + new._repetition = copy.deepcopy(self._repetition, memo) new._annotations = copy.deepcopy(self._annotations) return new diff --git a/masque/shapes/text.py b/masque/shapes/text.py index 93f2024..5047dc4 100644 --- a/masque/shapes/text.py +++ b/masque/shapes/text.py @@ -98,6 +98,7 @@ class Text(PositionableImpl, RotatableImpl, Shape): memo = {} if memo is None else memo new = copy.copy(self) new._offset = self._offset.copy() + new._repetition = copy.deepcopy(self._repetition, memo) new._annotations = copy.deepcopy(self._annotations) return new diff --git a/masque/test/test_pattern.py b/masque/test/test_pattern.py index 07e4150..6048bf1 100644 --- a/masque/test/test_pattern.py +++ b/masque/test/test_pattern.py @@ -1,13 +1,15 @@ import pytest +import copy from typing import cast from numpy.testing import assert_equal, assert_allclose from numpy import pi from ..error import PatternError +from ..abstract import Abstract from ..pattern import Pattern from ..shapes import Polygon from ..ref import Ref -from ..ports import Port +from ..ports import Port, PortError from ..label import Label from ..repetition import Grid @@ -133,6 +135,18 @@ def test_pattern_place_requires_abstract_for_reference() -> None: with pytest.raises(PatternError, match='Must provide an `Abstract`'): parent.place(child) + assert not parent.ports + + +def test_pattern_place_append_requires_pattern_atomically() -> None: + parent = Pattern() + child = Abstract("child", {"A": Port((1, 2), 0)}) + + with pytest.raises(PatternError, match='Must provide a full `Pattern`'): + parent.place(child, append=True) + + assert not parent.ports + def test_pattern_interface() -> None: source = Pattern() @@ -148,3 +162,81 @@ def test_pattern_interface() -> None: assert_allclose(iface.ports["out_A"].rotation, 0, atol=1e-10) assert iface.ports["in_A"].ptype == "test" assert iface.ports["out_A"].ptype == "test" + + +def test_pattern_interface_duplicate_port_map_targets_raise() -> None: + source = Pattern() + source.ports["A"] = Port((10, 20), 0) + source.ports["B"] = Port((30, 40), pi) + + with pytest.raises(PortError, match='Duplicate targets in `port_map`'): + Pattern.interface(source, port_map={"A": "X", "B": "X"}) + + +def test_pattern_interface_empty_port_map_copies_no_ports() -> None: + source = Pattern() + source.ports["A"] = Port((10, 20), 0) + source.ports["B"] = Port((30, 40), pi) + + assert not Pattern.interface(source, port_map={}).ports + assert not Pattern.interface(source, port_map=[]).ports + + +def test_pattern_plug_requires_abstract_for_reference_atomically() -> None: + parent = Pattern(ports={"X": Port((0, 0), 0)}) + child = Pattern(ports={"A": Port((0, 0), pi)}) + + with pytest.raises(PatternError, match='Must provide an `Abstract`'): + parent.plug(child, {"X": "A"}) + + assert set(parent.ports) == {"X"} + + +def test_pattern_append_port_conflict_is_atomic() -> None: + pat1 = Pattern() + pat1.ports["A"] = Port((0, 0), 0) + + pat2 = Pattern() + pat2.polygon((1, 0), vertices=[[0, 0], [1, 0], [0, 1]]) + pat2.ports["A"] = Port((1, 0), 0) + + with pytest.raises(PatternError, match="Port names overlap"): + pat1.append(pat2) + + assert not pat1.shapes + assert set(pat1.ports) == {"A"} + + +def test_pattern_append_annotation_conflict_is_atomic() -> None: + pat1 = Pattern(annotations={"k": [1]}) + pat2 = Pattern(annotations={"k": [2]}) + pat2.polygon((1, 0), vertices=[[0, 0], [1, 0], [0, 1]]) + + with pytest.raises(PatternError, match="Annotation keys overlap"): + pat1.append(pat2) + + assert not pat1.shapes + assert pat1.annotations == {"k": [1]} + + +def test_pattern_deepcopy_does_not_share_shape_repetitions() -> None: + pat = Pattern() + pat.polygon((1, 0), vertices=[[0, 0], [1, 0], [0, 1]], repetition=Grid(a_vector=(10, 0), a_count=2)) + + pat2 = copy.deepcopy(pat) + pat2.scale_by(2) + + assert_allclose(cast("Polygon", pat.shapes[(1, 0)][0]).repetition.a_vector, [10, 0]) + assert_allclose(cast("Polygon", pat2.shapes[(1, 0)][0]).repetition.a_vector, [20, 0]) + + +def test_pattern_flatten_does_not_mutate_child_repetitions() -> None: + child = Pattern() + child.polygon((1, 0), vertices=[[0, 0], [1, 0], [0, 1]], repetition=Grid(a_vector=(10, 0), a_count=2)) + + parent = Pattern() + parent.ref("child", scale=2) + + parent.flatten({"child": child}) + + assert_allclose(cast("Polygon", child.shapes[(1, 0)][0]).repetition.a_vector, [10, 0]) diff --git a/masque/test/test_ports.py b/masque/test/test_ports.py index 0291a1c..0f60809 100644 --- a/masque/test/test_ports.py +++ b/masque/test/test_ports.py @@ -89,6 +89,25 @@ def test_port_list_rename_missing_port_raises() -> None: assert set(pl.ports) == {"A"} +def test_port_list_rename_colliding_targets_raises() -> None: + class MyPorts(PortList): + def __init__(self) -> None: + self._ports = {"A": Port((0, 0), 0), "B": Port((1, 0), 0)} + + @property + def ports(self) -> dict[str, Port]: + return self._ports + + @ports.setter + def ports(self, val: dict[str, Port]) -> None: + self._ports = val + + pl = MyPorts() + with pytest.raises(PortError, match="Renamed ports would collide"): + pl.rename_ports({"A": "C", "B": "C"}) + assert set(pl.ports) == {"A", "B"} + + def test_port_list_add_port_pair_requires_distinct_names() -> None: class MyPorts(PortList): def __init__(self) -> None: @@ -163,9 +182,30 @@ def test_port_list_plugged_missing_port_raises() -> None: pl.plugged({"missing": "B"}) assert set(pl.ports) == {"A", "B"} + +def test_port_list_plugged_reused_port_raises_atomically() -> None: + class MyPorts(PortList): + def __init__(self) -> None: + self._ports = {"A": Port((0, 0), None), "B": Port((0, 0), None), "C": Port((0, 0), None)} + + @property + def ports(self) -> dict[str, Port]: + return self._ports + + @ports.setter + def ports(self, val: dict[str, Port]) -> None: + self._ports = val + + for connections in ({"A": "A"}, {"A": "B", "C": "B"}): + pl = MyPorts() + with pytest.raises(PortError, match="Each port may appear in at most one connection"): + pl.plugged(connections) + assert set(pl.ports) == {"A", "B", "C"} + + pl = MyPorts() with pytest.raises(PortError, match="Connection destination ports were not found"): pl.plugged({"A": "missing"}) - assert set(pl.ports) == {"A", "B"} + assert set(pl.ports) == {"A", "B", "C"} def test_port_list_plugged_mismatch() -> None: diff --git a/masque/test/test_shape_advanced.py b/masque/test/test_shape_advanced.py index fc76fcb..2dec264 100644 --- a/masque/test/test_shape_advanced.py +++ b/masque/test/test_shape_advanced.py @@ -122,6 +122,21 @@ def test_curve_polygonizers_clamp_large_max_arclen() -> None: assert len(polys[0].vertices) >= 3 +def test_ellipse_integer_radii_scale_cleanly() -> None: + ellipse = Ellipse(radii=(10, 20)) + ellipse.scale_by(0.5) + assert_allclose(ellipse.radii, [5, 10]) + + +def test_arc_rejects_zero_radii_up_front() -> None: + with pytest.raises(PatternError, match='Radii must be positive'): + Arc(radii=(0, 5), angles=(0, 1), width=1) + with pytest.raises(PatternError, match='Radii must be positive'): + Arc(radii=(5, 0), angles=(0, 1), width=1) + with pytest.raises(PatternError, match='Radii must be positive'): + Arc(radii=(0, 0), angles=(0, 1), width=1) + + def test_path_edge_cases() -> None: # Zero-length segments p = MPath(vertices=[[0, 0], [0, 0], [10, 0]], width=2)