From 4e8a911ffacaef55f0ae6c2077f363d80273a02e Mon Sep 17 00:00:00 2001 From: David Chau Date: Fri, 10 Jan 2025 11:50:20 -0500 Subject: [PATCH] Clean up transfer_liquid implementation for argument-passing hygiene: - Functions should not modify any argument that's passed in (unless that's the purpose of the function). - Objects should not modify any arguments given to their constructor (unless the object is supposed to take ownership of the argument). --- .../protocol_api/core/engine/instrument.py | 32 +++++++++---------- .../engine/transfer_components_executor.py | 3 +- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/api/src/opentrons/protocol_api/core/engine/instrument.py b/api/src/opentrons/protocol_api/core/engine/instrument.py index 8ae164210dd..53c534561e5 100644 --- a/api/src/opentrons/protocol_api/core/engine/instrument.py +++ b/api/src/opentrons/protocol_api/core/engine/instrument.py @@ -1126,7 +1126,6 @@ def aspirate_liquid_class( Return: List of liquid and air gap pairs in tip. """ aspirate_props = transfer_properties.aspirate - _tip_contents = tip_contents.copy() tx_commons.check_valid_volume_parameters( disposal_volume=0, # No disposal volume for 1-to-1 transfer air_gap=aspirate_props.retract.air_gap_by_volume.get_for_volume(volume), @@ -1141,14 +1140,14 @@ def aspirate_liquid_class( ) ) aspirate_location = Location(aspirate_point, labware=source_loc.labware) - if len(_tip_contents) > 0: - last_liquid_and_airgap_in_tip = _tip_contents[-1] - else: - last_liquid_and_airgap_in_tip = tx_comps_executor.LiquidAndAirGapPair( + last_liquid_and_airgap_in_tip = ( + tip_contents[-1] + if tip_contents + else tx_comps_executor.LiquidAndAirGapPair( liquid=0, air_gap=0, ) - _tip_contents = [last_liquid_and_airgap_in_tip] + ) components_executor = tx_comps_executor.TransferComponentsExecutor( instrument_core=self, transfer_properties=transfer_properties, @@ -1170,9 +1169,11 @@ def aspirate_liquid_class( ) components_executor.aspirate_and_wait(volume=volume) components_executor.retract_after_aspiration(volume=volume) + + # return copy of tip_contents with last entry replaced by tip state from executor last_contents = components_executor.tip_state.last_liquid_and_air_gap_in_tip - _tip_contents[-1] = last_contents - return _tip_contents + new_tip_contents = tip_contents[0:-1] + [last_contents] + return new_tip_contents def dispense_liquid_class( self, @@ -1216,7 +1217,6 @@ def dispense_liquid_class( """ dispense_props = transfer_properties.dispense dest_loc, dest_well = dest - _tip_contents = tip_contents.copy() dispense_point = ( tx_comps_executor.absolute_point_from_position_reference_and_offset( well=dest_well, @@ -1225,14 +1225,14 @@ def dispense_liquid_class( ) ) dispense_location = Location(dispense_point, labware=dest_loc.labware) - if len(_tip_contents) > 0: - last_liquid_and_airgap_in_tip = _tip_contents[-1] - else: - last_liquid_and_airgap_in_tip = tx_comps_executor.LiquidAndAirGapPair( + last_liquid_and_airgap_in_tip = ( + tip_contents[-1] + if tip_contents + else tx_comps_executor.LiquidAndAirGapPair( liquid=0, air_gap=0, ) - _tip_contents = [last_liquid_and_airgap_in_tip] + ) components_executor = tx_comps_executor.TransferComponentsExecutor( instrument_core=self, transfer_properties=transfer_properties, @@ -1263,8 +1263,8 @@ def dispense_liquid_class( source_well=source[1] if source else None, ) last_contents = components_executor.tip_state.last_liquid_and_air_gap_in_tip - _tip_contents[-1] = last_contents - return _tip_contents + new_tip_contents = tip_contents[0:-1] + [last_contents] + return new_tip_contents def retract(self) -> None: """Retract this instrument to the top of the gantry.""" diff --git a/api/src/opentrons/protocol_api/core/engine/transfer_components_executor.py b/api/src/opentrons/protocol_api/core/engine/transfer_components_executor.py index c483ffc0863..b89f809bfd2 100644 --- a/api/src/opentrons/protocol_api/core/engine/transfer_components_executor.py +++ b/api/src/opentrons/protocol_api/core/engine/transfer_components_executor.py @@ -1,6 +1,7 @@ """Executor for liquid class based complex commands.""" from __future__ import annotations +from copy import deepcopy from enum import Enum from typing import TYPE_CHECKING, Optional, Union from dataclasses import dataclass, field @@ -105,7 +106,7 @@ def __init__( self._transfer_properties = transfer_properties self._target_location = target_location self._target_well = target_well - self._tip_state: TipState = tip_state + self._tip_state: TipState = deepcopy(tip_state) # don't modify caller's object self._transfer_type: TransferType = transfer_type @property