Skip to content

Commit e161ab1

Browse files
refactor(api): Move more vector arithmetic from Pydantic models to Point (#18699)
## Changelog * Delete arithmetic methods like `__add__()` from Pydantic models like `LabwareOffsetVector`. Now, callers should convert the model to a `Point` and use `Point`'s arithmetic methods. * To make it easier to convert to `Point`, instead of defining `to_point()` on every vector-like Pydantic model, add a `Point.from_xyz_attrs(model)` method that works on anything with `x`/`y`/`z` attributes. Delete the existing `to_point()` methods. The intent is to encourage us to consistently use `Point`s for everything mathematical, and reserve Pydantic models for just the interface boundaries where we're converting to/from JSON. I think we want to do this because: * We have a *lot* of vector-like Pydantic models (`LabwareOffsetVector`, `Vec3f`, `Vector3D`, etc.). We are working to consolidate these, but I doubt that we'll ever have one Pydantic vector model to rule them all. In part, because parsing/serialization contexts have different needs. Sometimes we want the fields to have defaults, sometimes we don't; sometimes we care about float vs. int, sometimes we don't. * Some of these Pydantic models, in particular in `protocol_engine/types/module.py`, really ought to live in `shared-data`. We can't move them to `shared-data` if they have a dependency on `opentrons.types.Point`.
1 parent 8ca3abe commit e161ab1

File tree

16 files changed

+197
-244
lines changed

16 files changed

+197
-244
lines changed

api/src/opentrons/protocol_engine/commands/absorbance_reader/close_lid.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
from ...state.update_types import StateUpdate
1414

15+
from opentrons.types import Point
1516

1617
from opentrons.drivers.types import AbsorbanceReaderLidStatus
1718

@@ -113,7 +114,10 @@ async def execute(self, params: CloseLidParams) -> SuccessData[CloseLidResult]:
113114
labware_definition=lid_definition,
114115
current_location=current_location,
115116
new_location=new_location,
116-
user_offset_data=lid_gripper_offsets,
117+
user_pick_up_offset=Point.from_xyz_attrs(
118+
lid_gripper_offsets.pickUpOffset
119+
),
120+
user_drop_offset=Point.from_xyz_attrs(lid_gripper_offsets.dropOffset),
117121
post_drop_slide_offset=None,
118122
)
119123
state_update.set_absorbance_reader_lid(

api/src/opentrons/protocol_engine/commands/absorbance_reader/open_lid.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from ...errors import CannotPerformModuleAction
1111

1212
from opentrons.protocol_engine.types import AddressableAreaLocation
13+
from opentrons.types import Point
1314

1415
from opentrons.drivers.types import AbsorbanceReaderLidStatus
1516

@@ -115,7 +116,10 @@ async def execute(self, params: OpenLidParams) -> SuccessData[OpenLidResult]:
115116
labware_definition=lid_definition,
116117
current_location=current_location,
117118
new_location=new_location,
118-
user_offset_data=lid_gripper_offsets,
119+
user_pick_up_offset=Point.from_xyz_attrs(
120+
lid_gripper_offsets.pickUpOffset
121+
),
122+
user_drop_offset=Point.from_xyz_attrs(lid_gripper_offsets.dropOffset),
119123
post_drop_slide_offset=None,
120124
)
121125
state_update.set_absorbance_reader_lid(

api/src/opentrons/protocol_engine/commands/move_labware.py

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
AddressableAreaLocation,
3636
LabwareMovementStrategy,
3737
LabwareOffsetVector,
38-
LabwareMovementOffsetData,
3938
LabwareLocationSequence,
4039
NotOnDeckLocationSequenceComponent,
4140
OFF_DECK_LOCATION,
@@ -186,7 +185,7 @@ async def execute(self, params: MoveLabwareParams) -> _ExecuteReturn: # noqa: C
186185
)
187186
definition_uri = current_labware.definitionUri
188187
post_drop_slide_offset: Optional[Point] = None
189-
trash_lid_drop_offset: Optional[LabwareOffsetVector] = None
188+
trash_lid_drop_offset: Optional[Point] = None
190189

191190
if self._state_view.labware.is_fixed_trash(params.labwareId):
192191
raise LabwareMovementNotAllowedError(
@@ -243,16 +242,14 @@ async def execute(self, params: MoveLabwareParams) -> _ExecuteReturn: # noqa: C
243242
if labware_validation.validate_definition_is_lid(
244243
self._state_view.labware.get_definition(params.labwareId)
245244
):
246-
lid_disposable_offfets = (
245+
lid_disposable_offsets = (
247246
current_labware_definition.gripperOffsets.get(
248247
"lidDisposalOffsets"
249248
)
250249
)
251-
if lid_disposable_offfets is not None:
252-
trash_lid_drop_offset = LabwareOffsetVector(
253-
x=lid_disposable_offfets.dropOffset.x,
254-
y=lid_disposable_offfets.dropOffset.y,
255-
z=lid_disposable_offfets.dropOffset.z,
250+
if lid_disposable_offsets is not None:
251+
trash_lid_drop_offset = Point.from_xyz_attrs(
252+
lid_disposable_offsets.dropOffset
256253
)
257254
else:
258255
raise LabwareOffsetDoesNotExistError(
@@ -354,13 +351,20 @@ async def execute(self, params: MoveLabwareParams) -> _ExecuteReturn: # noqa: C
354351
validated_new_loc = self._state_view.geometry.ensure_valid_gripper_location(
355352
available_new_location,
356353
)
357-
user_offset_data = LabwareMovementOffsetData(
358-
pickUpOffset=params.pickUpOffset or LabwareOffsetVector(x=0, y=0, z=0),
359-
dropOffset=params.dropOffset or LabwareOffsetVector(x=0, y=0, z=0),
354+
355+
user_pick_up_offset = (
356+
Point.from_xyz_attrs(params.pickUpOffset)
357+
if params.pickUpOffset is not None
358+
else Point()
359+
)
360+
user_drop_offset = (
361+
Point.from_xyz_attrs(params.dropOffset)
362+
if params.dropOffset is not None
363+
else Point()
360364
)
361365

362366
if trash_lid_drop_offset:
363-
user_offset_data.dropOffset += trash_lid_drop_offset
367+
user_drop_offset += trash_lid_drop_offset
364368

365369
immediate_destination_location_sequence = (
366370
self._state_view.geometry.get_predicted_location_sequence(
@@ -378,7 +382,8 @@ async def execute(self, params: MoveLabwareParams) -> _ExecuteReturn: # noqa: C
378382
labware_id=params.labwareId,
379383
current_location=validated_current_loc,
380384
new_location=validated_new_loc,
381-
user_offset_data=user_offset_data,
385+
user_pick_up_offset=user_pick_up_offset,
386+
user_drop_offset=user_drop_offset,
382387
post_drop_slide_offset=post_drop_slide_offset,
383388
)
384389
except (

api/src/opentrons/protocol_engine/execution/labware_movement.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
from ..types import (
3131
OnLabwareLocation,
3232
LabwareLocation,
33-
LabwareMovementOffsetData,
3433
OnDeckLabwareLocation,
3534
)
3635

@@ -96,7 +95,8 @@ async def move_labware_with_gripper(
9695
labware_id: str,
9796
current_location: OnDeckLabwareLocation,
9897
new_location: OnDeckLabwareLocation,
99-
user_offset_data: LabwareMovementOffsetData,
98+
user_pick_up_offset: Point,
99+
user_drop_offset: Point,
100100
post_drop_slide_offset: Optional[Point],
101101
) -> None:
102102
...
@@ -108,7 +108,8 @@ async def move_labware_with_gripper(
108108
labware_definition: LabwareDefinition,
109109
current_location: OnDeckLabwareLocation,
110110
new_location: OnDeckLabwareLocation,
111-
user_offset_data: LabwareMovementOffsetData,
111+
user_pick_up_offset: Point,
112+
user_drop_offset: Point,
112113
post_drop_slide_offset: Optional[Point],
113114
) -> None:
114115
...
@@ -120,7 +121,8 @@ async def move_labware_with_gripper( # noqa: C901
120121
labware_definition: LabwareDefinition | None = None,
121122
current_location: OnDeckLabwareLocation,
122123
new_location: OnDeckLabwareLocation,
123-
user_offset_data: LabwareMovementOffsetData,
124+
user_pick_up_offset: Point,
125+
user_drop_offset: Point,
124126
post_drop_slide_offset: Optional[Point],
125127
) -> None:
126128
"""Physically move a labware from one location to another using the gripper.
@@ -193,7 +195,8 @@ async def move_labware_with_gripper( # noqa: C901
193195
self._state_store.geometry.get_final_labware_movement_offset_vectors(
194196
from_location=current_location,
195197
to_location=new_location,
196-
additional_offset_vector=user_offset_data,
198+
additional_pick_up_offset=user_pick_up_offset,
199+
additional_drop_offset=user_drop_offset,
197200
current_labware=labware_definition,
198201
)
199202
)

api/src/opentrons/protocol_engine/state/_labware_origin_math.py

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
LabwareDefinition,
99
LabwareDefinition2,
1010
LabwareDefinition3,
11-
Vector3D,
1211
)
1312
from opentrons_shared_data.labware.types import (
1413
SlotFootprintAsChildFeature,
@@ -18,7 +17,6 @@
1817
LabwareParentDefinition,
1918
ModuleDefinition,
2019
ModuleModel,
21-
LabwareOffsetVector,
2220
DeckLocationDefinition,
2321
LabwareLocation,
2422
ModuleLocation,
@@ -32,7 +30,7 @@
3230
def get_parent_placement_origin_to_lw_origin(
3331
child_labware: LabwareDefinition,
3432
parent_deck_item: ModuleDefinition,
35-
module_parent_to_child_offset: LabwareOffsetVector,
33+
module_parent_to_child_offset: Point,
3634
deck_definition: DeckDefinitionV5,
3735
is_topmost_labware: bool,
3836
labware_location: ModuleLocation,
@@ -67,7 +65,7 @@ def get_parent_placement_origin_to_lw_origin(
6765
def get_parent_placement_origin_to_lw_origin(
6866
child_labware: LabwareDefinition,
6967
parent_deck_item: LabwareParentDefinition,
70-
module_parent_to_child_offset: Union[LabwareOffsetVector, None],
68+
module_parent_to_child_offset: Union[Point, None],
7169
deck_definition: DeckDefinitionV5,
7270
is_topmost_labware: bool,
7371
labware_location: LabwareLocation,
@@ -95,7 +93,7 @@ def get_parent_placement_origin_to_lw_origin(
9593
# For compatibility with historical (buggy?) behavior,
9694
# we only consider it when the child labware is the topmost labware in a stackup.
9795
parent_deck_item_origin_to_child_labware_origin = (
98-
_to_point(child_labware.cornerOffsetFromSlot)
96+
Point.from_xyz_attrs(child_labware.cornerOffsetFromSlot)
9997
if is_topmost_labware
10098
else Point(0, 0, 0)
10199
)
@@ -120,7 +118,7 @@ def get_parent_placement_origin_to_lw_origin(
120118
def _get_parent_deck_item_origin_to_child_labware_placement_origin(
121119
child_labware: LabwareDefinition,
122120
parent_deck_item: LabwareParentDefinition,
123-
module_parent_to_child_offset: Union[LabwareOffsetVector, None],
121+
module_parent_to_child_offset: Union[Point, None],
124122
deck_definition: DeckDefinitionV5,
125123
labware_location: LabwareLocation,
126124
) -> Point:
@@ -140,9 +138,7 @@ def _get_parent_deck_item_origin_to_child_labware_placement_origin(
140138
)
141139
)
142140

143-
module_offset_point = _to_point_from_lw_offset_vector(
144-
module_parent_to_child_offset
145-
)
141+
module_offset_point = Point.from_xyz_attrs(module_parent_to_child_offset)
146142
return module_offset_point - child_labware_overlap_with_parent_deck_item
147143

148144
elif isinstance(labware_location, OnLabwareLocation):
@@ -189,7 +185,7 @@ def _get_child_labware_overlap_with_parent_labware(
189185
f"No default labware overlap specified for parent labware: {parent_labware_name}"
190186
)
191187
else:
192-
return _to_point(overlap)
188+
return Point.from_xyz_attrs(overlap)
193189

194190

195191
def _get_child_labware_overlap_with_parent_module(
@@ -207,7 +203,7 @@ def _get_child_labware_overlap_with_parent_module(
207203
else:
208204
return Point(x=0, y=0, z=0)
209205

210-
return _to_point(child_labware_overlap)
206+
return Point.from_xyz_attrs(child_labware_overlap)
211207

212208

213209
def _is_thermocycler_on_ot2(
@@ -223,16 +219,6 @@ def _is_thermocycler_on_ot2(
223219
)
224220

225221

226-
def _to_point(vector: Vector3D) -> Point:
227-
"""Convert a Vector3D to a Point."""
228-
return Point(x=vector.x, y=vector.y, z=vector.z)
229-
230-
231-
def _to_point_from_lw_offset_vector(offset_vector: LabwareOffsetVector) -> Point:
232-
"""Convert a LabwareOffsetVector to a Point."""
233-
return Point(x=offset_vector.x, y=offset_vector.y, z=offset_vector.z)
234-
235-
236222
def _get_back_left_bottom_position(labware: LabwareDefinition3) -> Point:
237223
"""Get the back left bottom position from a v3 labware definition."""
238224
footprint_as_child = _get_labware_footprint_as_child(labware)

0 commit comments

Comments
 (0)