From f1e25debecd452214723d77145e6adc2da9cf21d Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Tue, 31 Mar 2026 22:03:19 -0700 Subject: [PATCH 1/7] [Ellipse] force radii to float dtype --- masque/shapes/ellipse.py | 2 +- masque/test/test_shape_advanced.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/masque/shapes/ellipse.py b/masque/shapes/ellipse.py index 55ce9fd..49f6489 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: diff --git a/masque/test/test_shape_advanced.py b/masque/test/test_shape_advanced.py index fc76fcb..9f04b61 100644 --- a/masque/test/test_shape_advanced.py +++ b/masque/test/test_shape_advanced.py @@ -122,6 +122,12 @@ 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_path_edge_cases() -> None: # Zero-length segments p = MPath(vertices=[[0, 0], [0, 0], [10, 0]], width=2) From 2176d56b4c0bcf85af967d2f53a107070f0bca40 Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Tue, 31 Mar 2026 22:03:42 -0700 Subject: [PATCH 2/7] [Arc] Error out on zero radius --- masque/shapes/arc.py | 12 ++++++------ masque/test/test_shape_advanced.py | 9 +++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/masque/shapes/arc.py b/masque/shapes/arc.py index ab2e47b..e6d49cf 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 diff --git a/masque/test/test_shape_advanced.py b/masque/test/test_shape_advanced.py index 9f04b61..2dec264 100644 --- a/masque/test/test_shape_advanced.py +++ b/masque/test/test_shape_advanced.py @@ -128,6 +128,15 @@ def test_ellipse_integer_radii_scale_cleanly() -> None: 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) From 8d50f497f1ee82a03b354d3b2305c41aadc930e9 Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Tue, 31 Mar 2026 22:15:21 -0700 Subject: [PATCH 3/7] [repetition / others] copies should get their own repetitions --- masque/label.py | 1 + masque/shapes/arc.py | 1 + masque/shapes/circle.py | 1 + masque/shapes/ellipse.py | 1 + masque/shapes/path.py | 1 + masque/shapes/poly_collection.py | 1 + masque/shapes/polygon.py | 1 + masque/shapes/text.py | 1 + masque/test/test_pattern.py | 24 ++++++++++++++++++++++++ 9 files changed, 32 insertions(+) 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/shapes/arc.py b/masque/shapes/arc.py index e6d49cf..2adfe10 100644 --- a/masque/shapes/arc.py +++ b/masque/shapes/arc.py @@ -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 49f6489..38799da 100644 --- a/masque/shapes/ellipse.py +++ b/masque/shapes/ellipse.py @@ -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..e875641 100644 --- a/masque/test/test_pattern.py +++ b/masque/test/test_pattern.py @@ -1,4 +1,5 @@ import pytest +import copy from typing import cast from numpy.testing import assert_equal, assert_allclose from numpy import pi @@ -148,3 +149,26 @@ 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_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]) From 6a7b3b2259be5e80f7f7d9cd651c8ce311d33b7f Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Tue, 31 Mar 2026 22:15:48 -0700 Subject: [PATCH 4/7] [PorList] Error if multiple ports are renamed to the same name --- masque/ports.py | 4 ++++ masque/test/test_ports.py | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/masque/ports.py b/masque/ports.py index 3a67003..a02be1c 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: diff --git a/masque/test/test_ports.py b/masque/test/test_ports.py index 0291a1c..191387f 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: From 35b42c397b69714d01a88b730d7d894a9971e7ac Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Tue, 31 Mar 2026 22:37:16 -0700 Subject: [PATCH 5/7] [Pattern.append] don't dirty pattern if append() fails --- masque/pattern.py | 17 ++++++++++------- masque/test/test_pattern.py | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/masque/pattern.py b/masque/pattern.py index ab5f55a..287ca92 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 diff --git a/masque/test/test_pattern.py b/masque/test/test_pattern.py index e875641..59cc225 100644 --- a/masque/test/test_pattern.py +++ b/masque/test/test_pattern.py @@ -151,6 +151,33 @@ def test_pattern_interface() -> None: assert iface.ports["out_A"].ptype == "test" +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)) From 395ad4df9d4e98ccbf2c5eb4f8953a8705efb0cf Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Tue, 31 Mar 2026 22:38:27 -0700 Subject: [PATCH 6/7] [PortList] plugged() failure shouldn't dirty the ports --- masque/ports.py | 4 ++++ masque/test/test_ports.py | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/masque/ports.py b/masque/ports.py index a02be1c..8d7b6a6 100644 --- a/masque/ports.py +++ b/masque/ports.py @@ -413,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/test/test_ports.py b/masque/test/test_ports.py index 191387f..0f60809 100644 --- a/masque/test/test_ports.py +++ b/masque/test/test_ports.py @@ -182,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: From 9767ee4e625e0ff01fbc8eb6f3fe31a75bdb948a Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Tue, 31 Mar 2026 23:00:35 -0700 Subject: [PATCH 7/7] [Pattern] improve atomicity of place(), plug(), interface() --- masque/pattern.py | 31 +++++++++++++++++--------- masque/test/test_pattern.py | 43 ++++++++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/masque/pattern.py b/masque/pattern.py index 287ca92..b670493 100644 --- a/masque/pattern.py +++ b/masque/pattern.py @@ -1383,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) @@ -1404,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: @@ -1414,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) @@ -1554,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, @@ -1587,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, @@ -1662,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/test/test_pattern.py b/masque/test/test_pattern.py index 59cc225..6048bf1 100644 --- a/masque/test/test_pattern.py +++ b/masque/test/test_pattern.py @@ -5,10 +5,11 @@ 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 @@ -134,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() @@ -151,6 +164,34 @@ def test_pattern_interface() -> None: 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)