Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api): Allow omitting the mount when loading a 96-channel #14187

Merged
merged 6 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
32 changes: 21 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
Comment on lines -834 to -835
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure when this prefix thing was true, but it is not true now and it's weird, so I'm removing it from the docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚮

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,18 @@ 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
# Always validate the input mount, even if we're loading a 96-channel and the mount doesn't
# matter.
checked_mount = validation.ensure_mount(mount)

checked_mount = Mount.LEFT if is_96_channel else validation.ensure_mount(mount)
is_96_channel = checked_instrument_name == PipetteNameType.P1000_96
if is_96_channel:
checked_mount = Mount.LEFT
else:
if checked_mount is None:
raise ValueError(
f"You must specify a mount to load {repr(instrument_name)}."
)
SyntaxColoring marked this conversation as resolved.
Show resolved Hide resolved

tip_racks = tip_racks or []

Expand Down
6 changes: 4 additions & 2 deletions api/src/opentrons/protocol_api/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,11 @@ 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(mount: Union[str, Mount, None]) -> Optional[Mount]:
"""Ensure that an input value represents a valid Mount."""
if mount is None:
return None

if mount in [Mount.EXTENSION, "extension"]:
# This would cause existing protocols that might be iterating over mount types
# for loading pipettes to raise an error because Mount now includes Extension mount.
Expand Down Expand Up @@ -274,7 +277,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
13 changes: 13 additions & 0 deletions api/tests/opentrons/protocol_api/test_protocol_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,15 @@ def test_load_instrument(
)


def test_load_instrument_usually_requires_mount(
decoy: Decoy, subject: ProtocolContext
) -> None:
"""For pipettes other than 96-channels, it should require a mount."""
decoy.when(mock_validation.ensure_mount("some_mount")).then_return(None)
with pytest.raises(ValueError, match="You must specify a mount"):
subject.load_instrument("some_instrument", "some_mount")


def test_load_instrument_replace(
decoy: Decoy, mock_core: ProtocolCore, subject: ProtocolContext
) -> None:
Expand Down Expand Up @@ -241,6 +250,9 @@ def test_96_channel_pipette_always_loads_on_the_left_mount(
decoy.when(mock_validation.ensure_pipette_name("a 96 channel name")).then_return(
PipetteNameType.P1000_96
)
decoy.when(mock_validation.ensure_mount("shadowfax")).then_return(
Mount.RIGHT
) # Should be ignored.
decoy.when(
mock_core.load_instrument(
instrument_name=PipetteNameType.P1000_96,
Expand Down Expand Up @@ -295,6 +307,7 @@ 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("shadowfax")).then_return(None)
decoy.when(
mock_core.load_instrument(
instrument_name=PipetteNameType.P1000_96,
Expand Down
1 change: 1 addition & 0 deletions api/tests/opentrons/protocol_api/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
("LeFt", Mount.LEFT),
(Mount.LEFT, Mount.LEFT),
(Mount.RIGHT, Mount.RIGHT),
(None, None),
],
)
def test_ensure_mount(input_value: Union[str, Mount], expected: Mount) -> None:
Expand Down
Loading