Skip to content

Commit

Permalink
fix(api): simulation allows aspirating and dispensing on a tip rack (#…
Browse files Browse the repository at this point in the history
…7788)

* fix(api): simulation allows aspirating and dispensing on a tip rack
Doing the most basic fix and testing whether the destination is a tip-rack. I don't like it. It seems like there is a nicer and more general solution to validation problems. Though it looks like validation is often combined with type constraining. Will either put up a draft PR or pursue alternative:
- refactor `aspirate` to see if the destination is a tiprack and raise runtime error if it is.
- create a test protocol to unit test
- adding change to .gitignore to include package-lock.json as it does not seem to be in the project but is generated if one does `npm install` on the root.

* - refactor test protocol name so that it doesn't deviate from the convention already used (though no bug protocol scripts, I don't think)

* Refactoring my initial approach 'cause old man linter complained about complexity. And happily he did 'cause I totally disregarded dispense. Have since:
- moved validation to keep validation in instrument.py company
- unit test them
- fix linting issues

* - fix my comments

* - fix linting

* - addressing PR feedback and updating such that we constrain `validate_can_dispense` to locations as it cannot be anything other
- updating unit tests

* - bumping version as requested and documenting change

* - API version checking validation and have come full circle and am causing lint complexity issues. Silencing 'cause I don't feel like refactoring the method as there is no complexity introduced that the method has not already had a taste of.

* - addressing more feedback which pointed out that I am able to constrain types that may be aspirated and dispensed to `Location`

* - taking suggested change to change log

Co-authored-by: Mike Cousins <[email protected]>

* - update version to next release as suggested

Co-authored-by: Mike Cousins <[email protected]>

Co-authored-by: Mike Cousins <[email protected]>
  • Loading branch information
celsasser and mcous authored May 20, 2021
1 parent 6d11566 commit 1d624d8
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 4 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pip-delete-this-directory.txt

# node packages
node_modules
package-lock.json

# Unit test / coverage reports
htmlcov/
Expand Down
6 changes: 6 additions & 0 deletions api/docs/v2/versioning.rst
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ This table lists the correspondence between Protocol API versions and robot soft
+-------------+-----------------------------+
| 2.10 | 4.3.0 |
+-------------+-----------------------------+
| 2.11 | 4.4.0 |
+-------------+-----------------------------+


Changes in API Versions
Expand Down Expand Up @@ -207,3 +209,7 @@ Version 2.9
Version 2.10
++++++++++++
- In Python protocols requesting API version 2.10, moving to the same well twice in a row with different pipettes no longer results in strange diagonal movements.

Version 2.11
++++++++++++
- In Python protocols requesting API version 2.11, attempting to aspirate from or dispense to tip racks will raise an error.
8 changes: 6 additions & 2 deletions api/src/opentrons/protocol_api/instrument_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from opentrons.protocols.advanced_control.mix import mix_from_kwargs
from opentrons.protocols.api_support.instrument import \
validate_blowout_location, tip_length_for, validate_tiprack, \
determine_drop_target
determine_drop_target, validate_can_aspirate, validate_can_dispense
from opentrons.protocols.api_support.labware_like import LabwareLike
from opentrons.protocol_api.module_contexts import ThermocyclerContext
from opentrons.protocols.api_support.util import (
Expand Down Expand Up @@ -123,7 +123,7 @@ def default_speed(self) -> float:
def default_speed(self, speed: float):
self._implementation.set_default_speed(speed)

@requires_version(2, 0)
@requires_version(2, 0) # noqa: C901
def aspirate(self,
volume: Optional[float] = None,
location: Union[types.Location, Well] = None,
Expand Down Expand Up @@ -183,6 +183,8 @@ def aspirate(self,
" method that moves to a location (such as move_to or "
"dispense) must previously have been called so the robot "
"knows where it is.")
if self.api_version >= APIVersion(2, 11):
validate_can_aspirate(dest)

if self.current_volume == 0:
# Make sure we're at the top of the labware and clear of any
Expand Down Expand Up @@ -287,6 +289,8 @@ def dispense(self,
" method that moves to a location (such as move_to or "
"aspirate) must previously have been called so the robot "
"knows where it is.")
if self.api_version >= APIVersion(2, 11):
validate_can_dispense(loc)

c_vol = self.current_volume if not volume else volume

Expand Down
2 changes: 1 addition & 1 deletion api/src/opentrons/protocols/api_support/definitions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from .types import APIVersion

MAX_SUPPORTED_VERSION = APIVersion(2, 10)
MAX_SUPPORTED_VERSION = APIVersion(2, 11)
#: The maximum supported protocol API version in this release

V2_MODULE_DEF_VERSION = APIVersion(2, 3)
Expand Down
37 changes: 37 additions & 0 deletions api/src/opentrons/protocols/api_support/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,40 @@ def determine_drop_target(
assert tr.is_tiprack
z_height = return_height * tr.tip_length
return location.top(-z_height)


def validate_can_aspirate(location: types.Location) -> None:
""" Can one aspirate on the given `location` or not? This method is
pretty basic and will probably remain so (?) as the future holds neat
ambitions for how validation is implemented. And as robots become more
intelligent more rigorous testing will be possible
Args:
location: target for aspiration
Raises:
RuntimeError:
"""
if _is_tiprack(location):
raise RuntimeError("Cannot aspirate a tiprack")


def validate_can_dispense(location: types.Location) -> None:
""" Can one dispense to the given `location` or not? This method is
pretty basic and will probably remain so (?) as the future holds neat
ambitions for how validation is implemented. And as robots become more
intelligent more rigorous testing will be possible
Args:
location: target for dispense
Raises:
RuntimeError:
"""
if _is_tiprack(location):
raise RuntimeError("Cannot dispense to a tiprack")


def _is_tiprack(location: types.Location) -> bool:
labware = location.labware.as_labware()
return labware.parent and labware.parent.is_tiprack
22 changes: 22 additions & 0 deletions api/tests/opentrons/data/bug_aspirate_tip.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from opentrons import protocol_api

# metadata
metadata = {
'protocolName': 'Bug 7552',
'author': 'Name <[email protected]>',
'description': 'Simulation allows aspirating and dispensing on a tip rack',
'apiLevel': '2.11'
}


# protocol run function. the part after the colon lets your editor know
# where to look for autocomplete suggestions
def run(protocol: protocol_api.ProtocolContext):
# labware
plate = protocol.load_labware('geb_96_tiprack_10ul', 4)

# pipettes
pipette = protocol.load_instrument('p300_single', 'left', tip_racks=[plate])

# commands
pipette.transfer(5, plate.wells_by_name()['A1'], plate.wells_by_name()['B1'])
21 changes: 20 additions & 1 deletion api/tests/opentrons/protocols/api_support/test_instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import pytest
from opentrons.protocol_api.labware import Well
from opentrons.protocols.api_support.instrument import determine_drop_target
from opentrons.protocols.api_support.instrument import determine_drop_target, \
validate_can_aspirate, \
validate_can_dispense
from opentrons.protocols.geometry.well_geometry import WellGeometry
from opentrons.protocols.context.well import WellImplementation
from opentrons.protocols.api_support.types import APIVersion
Expand Down Expand Up @@ -48,3 +50,20 @@ def test_determine_drop_target(
r = determine_drop_target(api_version, well, 0.5)
assert r.labware.object == well
assert r.point == expected_point


def test_validate_can_aspirate(ctx):
well_plate = ctx.load_labware('corning_96_wellplate_360ul_flat', 1)
tip_rack = ctx.load_labware('opentrons_96_tiprack_300ul', 2)
# test type `Location`
validate_can_aspirate(well_plate.wells()[0].top())
with pytest.raises(RuntimeError):
validate_can_aspirate(tip_rack.wells_by_name()['A1'].top())


def test_validate_can_dispense(ctx):
well_plate = ctx.load_labware('corning_96_wellplate_360ul_flat', 1)
tip_rack = ctx.load_labware('opentrons_96_tiprack_300ul', 2)
validate_can_dispense(well_plate.wells()[0].top())
with pytest.raises(RuntimeError):
validate_can_dispense(tip_rack.wells_by_name()['A1'].top())
6 changes: 6 additions & 0 deletions api/tests/opentrons/test_simulate.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,9 @@ def test_simulate_extra_labware(protocol, protocol_file, monkeypatch):
ctx = simulate.get_protocol_api('2.0')
with pytest.raises(FileNotFoundError):
ctx.load_labware("fixture_12_trough", 1, namespace='fixture')


@pytest.mark.parametrize('protocol_file', ['bug_aspirate_tip.py'])
def test_simulate_aspirate_tip(protocol, protocol_file, monkeypatch):
with pytest.raises(ExceptionInProtocolError):
simulate.simulate(protocol.filelike, 'bug_aspirate_tip.py')

0 comments on commit 1d624d8

Please sign in to comment.