[Pattern] improve atomicity of place(), plug(), interface()

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
 

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)

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]
 

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:
+        if not val.min() > 0:
-            raise PatternError('Radii must be non-negative')
+            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:
+        if not val > 0:
-            raise PatternError('Radius must be non-negative')
+            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:
+        if not val > 0:
-            raise PatternError('Radius must be non-negative')
+            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
 

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
 

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
 

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
 

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
 

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
 

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
 

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])

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:

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)