Skip to content

Commit c330ba1

Browse files
authored
feat: improve position validation for absolute/relative grid plans (#263)
* feat: update validations * update test * refactor validations * update test + comment * refactor position validation * remove unused * update position addition * update error message * revert * update tests * update tests * update test * fix import * test: fix * feat: warning for ignored position with global absolute grid + fix tests
1 parent 0829938 commit c330ba1

6 files changed

Lines changed: 244 additions & 17 deletions

File tree

src/useq/_mda_sequence.py

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from useq._hardware_autofocus import AnyAutofocusPlan, AxesBasedAF
2121
from useq._iter_sequence import iter_sequence
2222
from useq._plate import WellPlatePlan
23-
from useq._position import Position, PositionBase
23+
from useq._position import Position, PositionBase, RelativePosition
2424
from useq._time import AnyTimePlan # noqa: TC001
2525
from useq._utils import TimeEstimate, estimate_sequence_duration
2626
from useq._z import AnyZPlan # noqa: TC001
@@ -319,6 +319,12 @@ def _validate_stage_positions(
319319

320320
positions = []
321321
for v in value:
322+
if isinstance(v, RelativePosition):
323+
raise ValueError(
324+
"RelativePosition cannot be used in stage_positions. "
325+
"Use AbsolutePosition (Position)) instead. For z-only "
326+
"positions, use Position(z=<value>)."
327+
)
322328
if isinstance(v, Position):
323329
positions.append(v)
324330
elif isinstance(v, dict):
@@ -371,6 +377,31 @@ def _validate_mda(self) -> Self:
371377
"keep_shutter_open_across cannot currently be set on a "
372378
"Position sequence"
373379
)
380+
381+
# warn and clear x/y on positions when using a global absolute grid
382+
if self.grid_plan is not None and not self.grid_plan.is_relative:
383+
new_positions = list(self.stage_positions)
384+
for i, p in enumerate(new_positions):
385+
# skip positions with their own sub-sequence grid;
386+
# x/y serves as the origin for that grid
387+
if p.sequence is not None and p.sequence.grid_plan:
388+
continue
389+
if p.x is not None or p.y is not None:
390+
import warnings
391+
392+
warnings.warn(
393+
f"Position x={p.x!r}, y={p.y!r} is ignored when "
394+
f"using a global absolute grid plan "
395+
f"({type(self.grid_plan).__name__}). "
396+
"Set x=None, y=None on the position to silence "
397+
"this warning. In a future version this will raise "
398+
"an error.",
399+
UserWarning,
400+
stacklevel=2,
401+
)
402+
new_positions[i] = p.model_copy(update={"x": None, "y": None})
403+
object.__setattr__(self, "stage_positions", tuple(new_positions))
404+
374405
return self
375406

376407
def __eq__(self, other: Any) -> bool:

src/useq/_position.py

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import warnings
12
from collections.abc import Iterator
23
from typing import TYPE_CHECKING, Any, Generic, Optional, SupportsIndex, TypeVar
34

@@ -52,17 +53,25 @@ def __add__(self, other: "RelativePosition") -> "Self":
5253
"""Add two positions together to create a new position."""
5354
if not isinstance(other, RelativePosition): # pragma: no cover
5455
return NotImplemented
55-
if (x := self.x) is not None and other.x is not None:
56-
x += other.x
57-
if (y := self.y) is not None and other.y is not None:
58-
y += other.y
59-
if (z := self.z) is not None and other.z is not None:
60-
z += other.z
56+
if self.x is not None and other.x is not None:
57+
x = self.x + other.x
58+
else:
59+
x = self.x if other.x is None else other.x
60+
if self.y is not None and other.y is not None:
61+
y = self.y + other.y
62+
else:
63+
y = self.y if other.y is None else other.y
64+
if self.z is not None and other.z is not None:
65+
z = self.z + other.z
66+
else:
67+
z = self.z if other.z is None else other.z
6168
if (name := self.name) and other.name:
6269
name = f"{name}_{other.name}"
6370
kwargs = {**self.model_dump(), "x": x, "y": y, "z": z, "name": name}
6471
return type(self).model_construct(**kwargs) # type: ignore [return-value]
6572

73+
__radd__ = __add__
74+
6675
def __round__(self, ndigits: "SupportsIndex | None" = None) -> "Self":
6776
"""Round the position to the given number of decimal places."""
6877
kwargs = {
@@ -96,6 +105,28 @@ class AbsolutePosition(PositionBase, FrozenModel):
96105
def is_relative(self) -> bool:
97106
return False
98107

108+
@model_validator(mode="after")
109+
def _validate_position(self) -> "Self":
110+
if self.sequence is None or self.sequence.grid_plan is None:
111+
return self
112+
grid = self.sequence.grid_plan
113+
if not grid.is_relative:
114+
# x/y are meaningless with an absolute sub-grid (the grid defines
115+
# them). Warn and clear.
116+
if self.x is not None or self.y is not None:
117+
warnings.warn(
118+
f"Position x={self.x!r}, y={self.y!r} is ignored when a position "
119+
f"sequence uses an absolute grid plan ({type(grid).__name__}). "
120+
"Set x=None, y=None on the position to silence this warning. "
121+
"In a future version this will raise an error.",
122+
UserWarning,
123+
stacklevel=2,
124+
)
125+
object.__setattr__(self, "x", None)
126+
object.__setattr__(self, "y", None)
127+
128+
return self
129+
99130

100131
Position = AbsolutePosition # for backwards compatibility
101132
PositionT = TypeVar("PositionT", bound=PositionBase)

tests/fixtures/cases.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@ def genindex(axes: dict[str, int]) -> list[dict[str, int]]:
172172
stage_positions=[
173173
Position(x=0, y=0),
174174
Position(
175-
x=10,
176-
y=10,
175+
x=None,
176+
y=None,
177177
sequence={
178178
"grid_plan": GridFromEdges(top=1, bottom=-1, left=0, right=0)
179179
},
@@ -303,8 +303,12 @@ def genindex(axes: dict[str, int]) -> list[dict[str, int]]:
303303
name="multi_g_in_position_sub_sequence",
304304
seq=MDASequence(
305305
stage_positions=[
306-
{"sequence": {"grid_plan": {"rows": 1, "columns": 2}}},
307-
{"sequence": {"grid_plan": GridRowsColumns(rows=2, columns=2)}},
306+
{"x": 0, "y": 0, "sequence": {"grid_plan": {"rows": 1, "columns": 2}}},
307+
{
308+
"x": 0,
309+
"y": 0,
310+
"sequence": {"grid_plan": GridRowsColumns(rows=2, columns=2)},
311+
},
308312
{
309313
"sequence": {
310314
"grid_plan": GridFromEdges(top=1, bottom=-1, left=0, right=0)
@@ -602,7 +606,7 @@ def genindex(axes: dict[str, int]) -> list[dict[str, int]]:
602606
seq=MDASequence(
603607
axis_order="tpgcz",
604608
stage_positions=[
605-
Position(x=0, y=0),
609+
Position(z=2),
606610
Position(
607611
name="name",
608612
x=10,
@@ -886,7 +890,7 @@ def _pred(events: Sequence[MDAEvent]) -> str | None:
886890
MDATestCase(
887891
name="af_axes_g_basic",
888892
seq=MDASequence(
889-
stage_positions=[Position(z=30)],
893+
stage_positions=[Position(x=0, y=0, z=30)],
890894
channels=["DAPI", "FITC"],
891895
grid_plan=GridRowsColumns(rows=2, columns=1),
892896
autofocus_plan=AxesBasedAF(autofocus_device_name="Z", axes=("g",)),
@@ -1126,7 +1130,7 @@ def _pred(events: Sequence[MDAEvent]) -> str | None:
11261130
channels=["DAPI", "FITC"],
11271131
stage_positions=[
11281132
Position(
1129-
sequence=MDASequence(grid_plan=GridRowsColumns(rows=2, columns=2))
1133+
sequence=MDASequence(grid_plan=GridRowsColumns(rows=2, columns=2)),
11301134
)
11311135
],
11321136
keep_shutter_open_across="g",

tests/test_sequence.py

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
CameraROI,
1212
MDAEvent,
1313
MDASequence,
14+
Position,
1415
TIntervalDuration,
1516
ZAboveBelow,
1617
ZRangeAround,
@@ -72,7 +73,7 @@ def test_axis_order_errors() -> None:
7273

7374
with pytest.warns(UserWarning, match="Global grid plan will override"):
7475
MDASequence(
75-
stage_positions=[(0, 0, 0), (10, 10, 10)],
76+
stage_positions=[{"z": 0}, {"z": 10}],
7677
grid_plan={"top": 1, "bottom": -1, "left": 0, "right": 0},
7778
)
7879

@@ -85,7 +86,7 @@ def test_axis_order_errors() -> None:
8586
# if all but one sub-position has a grid plan , is ok
8687
MDASequence(
8788
stage_positions=[
88-
(0, 0, 0),
89+
{"z": 0},
8990
{"sequence": {"grid_plan": {"rows": 2, "columns": 2}}},
9091
{
9192
"sequence": {
@@ -104,6 +105,50 @@ def test_axis_order_errors() -> None:
104105
]
105106
)
106107

108+
# x/y on a position is ignored with a global absolute grid
109+
# --- GridFromEdges ---
110+
with pytest.warns(
111+
UserWarning, match="is ignored when using a global absolute grid plan"
112+
):
113+
seq = MDASequence(
114+
stage_positions=[{"x": 10, "y": 20}],
115+
grid_plan={"top": 1, "bottom": -1, "left": 0, "right": 0},
116+
)
117+
pos = seq.stage_positions[0]
118+
assert isinstance(pos, Position)
119+
assert pos.x is None
120+
assert pos.y is None
121+
# --- GridFromPolygon ---
122+
with pytest.warns(
123+
UserWarning, match="is ignored when using a global absolute grid plan"
124+
):
125+
seq = MDASequence(
126+
stage_positions=[{"x": 10, "y": 20}],
127+
grid_plan={
128+
"vertices": [(0, 0), (4, 0), (2, 4)],
129+
"fov_width": 2,
130+
"fov_height": 2,
131+
},
132+
)
133+
pos = seq.stage_positions[0]
134+
assert isinstance(pos, Position)
135+
assert pos.x is None
136+
assert pos.y is None
137+
138+
# no warning when x/y are None with absolute grids
139+
MDASequence(
140+
stage_positions=[{}],
141+
grid_plan={"top": 1, "bottom": -1, "left": 0, "right": 0},
142+
)
143+
MDASequence(
144+
stage_positions=[{}],
145+
grid_plan={
146+
"vertices": [(0, 0), (4, 0), (2, 4)],
147+
"fov_width": 2,
148+
"fov_height": 2,
149+
},
150+
)
151+
107152

108153
@pytest.mark.parametrize("cls", [MDASequence, MDAEvent])
109154
def test_schema(cls: BaseModel) -> None:

tests/test_stage_positions.py

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,117 @@
3030
def test_stage_positions(position: Any, pexpectation: Sequence[float]) -> None:
3131
position = useq.Position.model_validate(position)
3232
assert (position.x, position.y, position.z) == pexpectation
33+
34+
35+
_ABSOLUTE_GRID_PLANS = [
36+
pytest.param(
37+
{"top": 1, "bottom": -1, "left": 0, "right": 0},
38+
id="GridFromEdges",
39+
),
40+
pytest.param(
41+
{"vertices": [(0, 0), (4, 0), (2, 4)], "fov_width": 2, "fov_height": 2},
42+
id="GridFromPolygon",
43+
),
44+
]
45+
46+
_RELATIVE_GRID_PLANS = [
47+
pytest.param({"rows": 2, "columns": 2}, id="GridRowsColumns"),
48+
pytest.param(
49+
useq.RandomPoints(num_points=3, max_width=100, max_height=100),
50+
id="RandomPoints",
51+
),
52+
]
53+
54+
55+
@pytest.mark.parametrize("grid_plan", _ABSOLUTE_GRID_PLANS)
56+
def test_position_warns_on_absolute_sub_sequence_grid(
57+
grid_plan: dict,
58+
) -> None:
59+
"""Position clears x/y and warns at construction when sub-sequence uses an absolute grid."""
60+
with pytest.warns(UserWarning, match="is ignored when a position sequence uses"):
61+
pos = useq.Position(x=1, y=2, sequence={"grid_plan": grid_plan})
62+
assert pos.x is None
63+
assert pos.y is None
64+
65+
66+
@pytest.mark.parametrize("grid_plan", _RELATIVE_GRID_PLANS)
67+
def test_position_no_warn_on_relative_sub_sequence_grid(grid_plan: Any) -> None:
68+
"""Position keeps x/y when sub-sequence uses a relative grid."""
69+
pos = useq.Position(x=1, y=2, sequence={"grid_plan": grid_plan})
70+
assert pos.x == 1
71+
assert pos.y == 2
72+
73+
74+
# --- __add__ / __radd__ -----------------------------------------------------------
75+
76+
_ADD_CASES = [
77+
pytest.param(
78+
useq.Position(x=1, y=2, z=3),
79+
useq.RelativePosition(x=5, y=10, z=1),
80+
(6, 12, 4),
81+
id="both_have_values",
82+
),
83+
pytest.param(
84+
useq.Position(x=None, y=None, z=3),
85+
useq.RelativePosition(x=5, y=10, z=0),
86+
(5, 10, 3),
87+
id="none_falls_back_to_other",
88+
),
89+
]
90+
91+
92+
@pytest.mark.parametrize("pos, rel, expected", _ADD_CASES)
93+
def test_position_add(
94+
pos: useq.Position, rel: useq.RelativePosition, expected: tuple
95+
) -> None:
96+
result = pos + rel
97+
assert (result.x, result.y, result.z) == expected
98+
99+
100+
def test_position_radd() -> None:
101+
"""__radd__ supports reversed addition (RelativePosition + AbsolutePosition)."""
102+
pos = useq.Position(x=1, y=2, z=3)
103+
rel = useq.RelativePosition(x=5, y=10, z=0)
104+
result = rel + pos
105+
assert (result.x, result.y, result.z) == (6, 12, 3)
106+
107+
108+
# --- RelativePosition rejected in stage_positions --------------------------------
109+
110+
111+
def test_relative_position_rejected_in_stage_positions() -> None:
112+
"""RelativePosition is always rejected in stage_positions."""
113+
with pytest.raises(Exception, match="RelativePosition cannot be used"):
114+
useq.MDASequence(stage_positions=[useq.RelativePosition(x=1, y=2, z=3)])
115+
116+
117+
# --- Global grid + position interactions -----------------------------------------
118+
119+
120+
def test_warns_global_abs_grid_does_not_mutate_original_position() -> None:
121+
"""Clearing x/y for a global absolute grid must not mutate the original Position."""
122+
pos = useq.Position(x=1, y=2, z=3)
123+
with pytest.warns(UserWarning, match="is ignored when using"):
124+
seq = useq.MDASequence(
125+
stage_positions=[pos],
126+
grid_plan={"top": 1, "bottom": -1, "left": 0, "right": 0},
127+
)
128+
assert pos.x == 1 # original must be untouched
129+
assert pos.y == 2
130+
assert seq.stage_positions[0].x is None # sequence copy is updated
131+
assert seq.stage_positions[0].y is None
132+
133+
134+
@pytest.mark.parametrize("grid_plan", _ABSOLUTE_GRID_PLANS)
135+
def test_z_only_position_iterates_with_absolute_grid(grid_plan: dict) -> None:
136+
"""Position(x=None, y=None, z=3) iterates correctly: grid provides x/y, pos z."""
137+
seq = useq.MDASequence(
138+
stage_positions=[useq.Position(x=None, y=None, z=3)],
139+
grid_plan=grid_plan,
140+
)
141+
events = list(seq)
142+
assert len(events) > 0
143+
for event in events:
144+
assert event.x_pos is not None
145+
assert event.y_pos is not None
146+
assert event.z_pos == 3.0

tests/test_time_estimation.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,9 @@ def test_time_estimation_with_z_skips(seq: useq.MDASequence) -> None:
129129
channels=[DAPI_10],
130130
stage_positions=[
131131
(0, 0), # 0.01
132-
useq.Position(x=1, sequence=useq.MDASequence(grid_plan=GRID_4)), # 0.04
132+
useq.Position(
133+
x=1, y=0, sequence=useq.MDASequence(grid_plan=GRID_4)
134+
), # 0.04
133135
],
134136
): 0.05, # 0.01 + 0.04
135137
}

0 commit comments

Comments
 (0)