Skip to content

Commit

Permalink
feat(api): Allow omitting the mount when loading a 96-channel (#14187)
Browse files Browse the repository at this point in the history
Co-authored-by: Ed Cormany <[email protected]>
  • Loading branch information
SyntaxColoring and ecormany authored Dec 14, 2023
1 parent fa83dd3 commit ae8f637
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 69 deletions.
6 changes: 3 additions & 3 deletions api/docs/v2/new_pipette.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ If you're writing a protocol that uses the Flex Gripper, you might think that th
Loading a Flex 96-Channel Pipette
---------------------------------

This code sample loads the Flex 96-Channel Pipette. Because of its size, the Flex 96-Channel Pipette requires the left *and* right pipette mounts. You cannot use this pipette with 1- or 8-Channel Pipette in the same protocol or when these instruments are attached to the robot. To load the 96-Channel Pipette, specify its position as ``mount='left'`` as shown here:
This code sample loads the Flex 96-Channel Pipette. Because of its size, the Flex 96-Channel Pipette requires the left *and* right pipette mounts. You cannot use this pipette with 1- or 8-Channel Pipette in the same protocol or when these instruments are attached to the robot. When loading the 96-Channel Pipette, you can omit the ``mount`` argument from ``load_instrument()`` as shown here:

.. code-block:: python
def run(protocol: protocol_api.ProtocolContext):
left = protocol.load_instrument(
instrument_name='flex_96channel_1000', mount='left')
pipette = protocol.load_instrument(
instrument_name='flex_96channel_1000')
.. versionadded:: 2.15

Expand Down
25 changes: 14 additions & 11 deletions api/src/opentrons/protocol_api/protocol_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ def loaded_modules(self) -> Dict[int, ModuleTypes]:
def load_instrument(
self,
instrument_name: str,
mount: Union[Mount, str],
mount: Union[Mount, str, None] = None,
tip_racks: Optional[List[Labware]] = None,
replace: bool = False,
) -> InstrumentContext:
Expand All @@ -831,15 +831,16 @@ def load_instrument(
ensure that the correct instrument is attached in the specified
location.
:param str instrument_name: The name of the instrument model, or a
prefix. For instance, 'p10_single' may be
used to request a P10 single regardless of
the version.
:param mount: The mount in which this instrument should be attached.
:param str instrument_name: Which instrument you want to load. See :ref:`new-pipette-models`
for the valid values.
:param mount: The mount where this instrument should be attached.
This can either be an instance of the enum type
:py:class:`.types.Mount` or one of the strings `'left'`
and `'right'`.
:type mount: types.Mount or str
:py:class:`.types.Mount` or one of the strings ``"left"``
or ``"right"``. If you're loading a Flex 96-Channel Pipette
(``instrument_name="flex_96channel_1000"``), you can leave this unspecified,
since it always occupies both mounts; if you do specify a value, it will be
ignored.
:type mount: types.Mount or str or ``None``
:param tip_racks: A list of tip racks from which to pick tips if
:py:meth:`.InstrumentContext.pick_up_tip` is called
without arguments.
Expand All @@ -850,9 +851,11 @@ def load_instrument(
"""
instrument_name = validation.ensure_lowercase_name(instrument_name)
checked_instrument_name = validation.ensure_pipette_name(instrument_name)
is_96_channel = checked_instrument_name == PipetteNameType.P1000_96
checked_mount = validation.ensure_mount_for_pipette(
mount, checked_instrument_name
)

checked_mount = Mount.LEFT if is_96_channel else validation.ensure_mount(mount)
is_96_channel = checked_instrument_name == PipetteNameType.P1000_96

tip_racks = tip_racks or []

Expand Down
23 changes: 21 additions & 2 deletions api/src/opentrons/protocol_api/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,27 @@ class InvalidTrashBinLocationError(ValueError):
"""An error raised when attempting to load trash bins in invalid slots."""


def ensure_mount(mount: Union[str, Mount]) -> Mount:
def ensure_mount_for_pipette(
mount: Union[str, Mount, None], pipette: PipetteNameType
) -> Mount:
"""Ensure that an input value represents a valid mount, and is valid for the given pipette."""
if pipette == PipetteNameType.P1000_96:
# Always validate the raw mount input, even if the pipette is a 96-channel and we're not going
# to use the mount value.
if mount is not None:
_ensure_mount(mount)
# Internal layers treat the 96-channel as being on the left mount.
return Mount.LEFT
else:
if mount is None:
raise InvalidPipetteMountError(
f"You must specify a left or right mount to load {pipette.value}."
)
else:
return _ensure_mount(mount)


def _ensure_mount(mount: Union[str, Mount]) -> Mount:
"""Ensure that an input value represents a valid Mount."""
if mount in [Mount.EXTENSION, "extension"]:
# This would cause existing protocols that might be iterating over mount types
Expand Down Expand Up @@ -274,7 +294,6 @@ def ensure_module_model(load_name: str) -> ModuleModel:
def ensure_and_convert_trash_bin_location(
deck_slot: Union[int, str], api_version: APIVersion, robot_type: RobotType
) -> str:

"""Ensure trash bin load location is valid.
Also, convert the deck slot to a valid trash bin addressable area.
Expand Down
71 changes: 29 additions & 42 deletions api/tests/opentrons/protocol_api/test_protocol_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,15 @@ def test_load_instrument(
mock_instrument_core = decoy.mock(cls=InstrumentCore)
mock_tip_racks = [decoy.mock(cls=Labware), decoy.mock(cls=Labware)]

decoy.when(mock_validation.ensure_mount("shadowfax")).then_return(Mount.LEFT)
decoy.when(mock_validation.ensure_lowercase_name("Gandalf")).then_return("gandalf")
decoy.when(mock_validation.ensure_pipette_name("gandalf")).then_return(
PipetteNameType.P300_SINGLE
)
decoy.when(
mock_validation.ensure_mount_for_pipette(
"shadowfax", PipetteNameType.P300_SINGLE
)
).then_return(Mount.LEFT)

decoy.when(
mock_core.load_instrument(
Expand Down Expand Up @@ -197,13 +201,17 @@ def test_load_instrument_replace(
"""It should allow/disallow pipette replacement."""
mock_instrument_core = decoy.mock(cls=InstrumentCore)

decoy.when(mock_validation.ensure_lowercase_name("ada")).then_return("ada")
decoy.when(mock_validation.ensure_mount(matchers.IsA(Mount))).then_return(
Mount.RIGHT
decoy.when(mock_validation.ensure_lowercase_name(matchers.IsA(str))).then_return(
"ada"
)
decoy.when(mock_validation.ensure_pipette_name(matchers.IsA(str))).then_return(
PipetteNameType.P300_SINGLE
)
decoy.when(
mock_validation.ensure_mount_for_pipette(
matchers.IsA(Mount), matchers.IsA(PipetteNameType)
)
).then_return(Mount.RIGHT)
decoy.when(
mock_core.load_instrument(
instrument_name=matchers.IsA(PipetteNameType),
Expand All @@ -227,36 +235,6 @@ def test_load_instrument_replace(
subject.load_instrument(instrument_name="ada", mount=Mount.RIGHT)


def test_96_channel_pipette_always_loads_on_the_left_mount(
decoy: Decoy,
mock_core: ProtocolCore,
subject: ProtocolContext,
) -> None:
"""It should always load a 96-channel pipette on left mount, regardless of the mount arg specified."""
mock_instrument_core = decoy.mock(cls=InstrumentCore)

decoy.when(mock_validation.ensure_lowercase_name("A 96 Channel Name")).then_return(
"a 96 channel name"
)
decoy.when(mock_validation.ensure_pipette_name("a 96 channel name")).then_return(
PipetteNameType.P1000_96
)
decoy.when(
mock_core.load_instrument(
instrument_name=PipetteNameType.P1000_96,
mount=Mount.LEFT,
)
).then_return(mock_instrument_core)
decoy.when(mock_core.get_disposal_locations()).then_raise(
NoTrashDefinedError("No trash!")
)

result = subject.load_instrument(
instrument_name="A 96 Channel Name", mount="shadowfax"
)
assert result == subject.loaded_instruments["left"]


def test_96_channel_pipette_raises_if_another_pipette_attached(
decoy: Decoy,
mock_core: ProtocolCore,
Expand All @@ -265,13 +243,17 @@ def test_96_channel_pipette_raises_if_another_pipette_attached(
"""It should always raise when loading a 96-channel pipette when another pipette is attached."""
mock_instrument_core = decoy.mock(cls=InstrumentCore)

decoy.when(mock_validation.ensure_lowercase_name("ada")).then_return("ada")
decoy.when(mock_validation.ensure_pipette_name("ada")).then_return(
PipetteNameType.P300_SINGLE
)
decoy.when(mock_validation.ensure_mount(matchers.IsA(Mount))).then_return(
Mount.RIGHT
)
decoy.when(
mock_validation.ensure_lowercase_name("A Single Channel Name")
).then_return("a single channel name")
decoy.when(
mock_validation.ensure_pipette_name("a single channel name")
).then_return(PipetteNameType.P300_SINGLE)
decoy.when(
mock_validation.ensure_mount_for_pipette(
Mount.RIGHT, PipetteNameType.P300_SINGLE
)
).then_return(Mount.RIGHT)

decoy.when(
mock_core.load_instrument(
Expand All @@ -286,7 +268,9 @@ def test_96_channel_pipette_raises_if_another_pipette_attached(
NoTrashDefinedError("No trash!")
)

pipette_1 = subject.load_instrument(instrument_name="ada", mount=Mount.RIGHT)
pipette_1 = subject.load_instrument(
instrument_name="A Single Channel Name", mount=Mount.RIGHT
)
assert subject.loaded_instruments["right"] is pipette_1

decoy.when(mock_validation.ensure_lowercase_name("A 96 Channel Name")).then_return(
Expand All @@ -295,6 +279,9 @@ def test_96_channel_pipette_raises_if_another_pipette_attached(
decoy.when(mock_validation.ensure_pipette_name("a 96 channel name")).then_return(
PipetteNameType.P1000_96
)
decoy.when(
mock_validation.ensure_mount_for_pipette("shadowfax", PipetteNameType.P1000_96)
).then_return(Mount.LEFT)
decoy.when(
mock_core.load_instrument(
instrument_name=PipetteNameType.P1000_96,
Expand Down
45 changes: 34 additions & 11 deletions api/tests/opentrons/protocol_api/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,28 @@


@pytest.mark.parametrize(
["input_value", "expected"],
["input_mount", "input_pipette", "expected"],
[
("left", Mount.LEFT),
("right", Mount.RIGHT),
("LeFt", Mount.LEFT),
(Mount.LEFT, Mount.LEFT),
(Mount.RIGHT, Mount.RIGHT),
# Different string capitalizations:
("left", PipetteNameType.P300_MULTI_GEN2, Mount.LEFT),
("right", PipetteNameType.P300_MULTI_GEN2, Mount.RIGHT),
("LeFt", PipetteNameType.P300_MULTI_GEN2, Mount.LEFT),
# Passing in a Mount:
(Mount.LEFT, PipetteNameType.P300_MULTI_GEN2, Mount.LEFT),
(Mount.RIGHT, PipetteNameType.P300_MULTI_GEN2, Mount.RIGHT),
# Special handling for the 96-channel:
("left", PipetteNameType.P1000_96, Mount.LEFT),
("right", PipetteNameType.P1000_96, Mount.LEFT),
(None, PipetteNameType.P1000_96, Mount.LEFT),
],
)
def test_ensure_mount(input_value: Union[str, Mount], expected: Mount) -> None:
def test_ensure_mount(
input_mount: Union[str, Mount, None],
input_pipette: PipetteNameType,
expected: Mount,
) -> None:
"""It should properly map strings and mounts."""
result = subject.ensure_mount(input_value)
result = subject.ensure_mount_for_pipette(input_mount, input_pipette)
assert result == expected


Expand All @@ -48,18 +58,31 @@ def test_ensure_mount_input_invalid() -> None:
with pytest.raises(
subject.InvalidPipetteMountError, match="must be 'left' or 'right'"
):
subject.ensure_mount("oh no")
subject.ensure_mount_for_pipette("oh no", PipetteNameType.P300_MULTI_GEN2)

# Any mount is valid for the 96-Channel, but it needs to be a valid mount.
with pytest.raises(
subject.InvalidPipetteMountError, match="must be 'left' or 'right'"
):
subject.ensure_mount_for_pipette("oh no", PipetteNameType.P1000_96)

with pytest.raises(
subject.PipetteMountTypeError,
match="'left', 'right', or an opentrons.types.Mount",
):
subject.ensure_mount(42) # type: ignore[arg-type]
subject.ensure_mount_for_pipette(42, PipetteNameType.P300_MULTI_GEN2) # type: ignore[arg-type]

with pytest.raises(
subject.InvalidPipetteMountError, match="Use the left or right mounts instead"
):
subject.ensure_mount(Mount.EXTENSION)
subject.ensure_mount_for_pipette(
Mount.EXTENSION, PipetteNameType.P300_MULTI_GEN2
)

with pytest.raises(
subject.InvalidPipetteMountError, match="You must specify a left or right mount"
):
subject.ensure_mount_for_pipette(None, PipetteNameType.P300_MULTI_GEN2)


@pytest.mark.parametrize(
Expand Down

0 comments on commit ae8f637

Please sign in to comment.