From be9dbf95a4a3583d37ad4bba4fc78e94f357c714 Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Wed, 17 Jul 2024 15:47:13 -0400 Subject: [PATCH] addressing comments. removing WellIdentifier --- .../opentrons/protocol_engine/state/wells.py | 72 +++++++++++-------- api/src/opentrons/protocol_engine/types.py | 11 --- .../protocol_engine/state/test_well_store.py | 6 +- .../protocol_engine/state/test_well_view.py | 35 +++++---- 4 files changed, 67 insertions(+), 57 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/wells.py b/api/src/opentrons/protocol_engine/state/wells.py index 1942b6bcbde..5da00a6b5bb 100644 --- a/api/src/opentrons/protocol_engine/state/wells.py +++ b/api/src/opentrons/protocol_engine/state/wells.py @@ -1,4 +1,5 @@ """Basic well data state and store.""" +import pdb from dataclasses import dataclass from datetime import datetime from typing import Dict, List, Optional @@ -6,9 +7,9 @@ FailCommandAction, SucceedCommandAction, ) -from opentrons.protocol_engine.commands.liquid_probe import LiquidProbe +from opentrons.protocol_engine.commands.liquid_probe import LiquidProbeResult from opentrons.protocol_engine.commands.pipetting_common import LiquidNotFoundError -from opentrons.protocol_engine.types import WellIdentifier, LiquidHeightInfo +from opentrons.protocol_engine.types import LiquidHeightInfo from .abstract_store import HasState, HandlesActions from ..actions import Action @@ -19,7 +20,8 @@ class WellState: """State of all wells.""" - measured_liquid_heights: Dict[WellIdentifier, LiquidHeightInfo] + # Dict[Labware: Dict[Wellname: [Height,TimeRecorded]]] + measured_liquid_heights: Dict[str, Dict[str, LiquidHeightInfo]] class WellStore(HasState[WellState], HandlesActions): @@ -34,37 +36,38 @@ def __init__(self) -> None: def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" if isinstance(action, SucceedCommandAction): - self._handle_succeded_command(action.command) + self._handle_succeeded_command(action.command) if isinstance(action, FailCommandAction): self._handle_failed_command(action) - def _handle_succeded_command(self, command: Command) -> None: - if isinstance(command, LiquidProbe): - well = WellIdentifier( - labware_id=command.params.labwareId, well_name=command.params.wellName + def _handle_succeeded_command(self, command: Command) -> None: + if isinstance(command.result, LiquidProbeResult): + self._set_liquid_height( + labware_id=command.params.labwareId, + well_name=command.params.wellName, + height=command.result.z_position, + time=command.createdAt, ) - if command.result is None: - self._set_liquid_height(well=well, height=None, time=None) - else: - self._set_liquid_height( - well=well, - height=command.result.z_position, - time=command.createdAt, - ) def _handle_failed_command(self, action: FailCommandAction) -> None: if isinstance(action.error, LiquidNotFoundError): - self._set_liquid_height(0) + self._set_liquid_height( + labware_id=action.error.private.labware_id, + well_name=action.error.private.well_name, + height=0, + time=action.failed_at, + ) def _set_liquid_height( - self, well: WellIdentifier, height: Optional[float], time: Optional[datetime] + self, labware_id: str, well_name: str, height: float, time: datetime ) -> None: """Set the liquid height of the well.""" - if height is None or time is None: - del self._state.measured_liquid_heights[well] - else: - lhi = LiquidHeightInfo(height=height, last_measured=time) - self._state.measured_liquid_heights[well] = lhi + lhi = LiquidHeightInfo(height=height, last_measured=time) + try: + self._state.measured_liquid_heights[labware_id] + except KeyError: + self._state.measured_liquid_heights[labware_id] = {} + self._state.measured_liquid_heights[labware_id][well_name] = lhi class WellView(HasState[WellState]): @@ -82,23 +85,32 @@ def __init__(self, state: WellState) -> None: def get_all(self) -> List[LiquidHeightInfo]: """Get all well liquid heights.""" - return list(self._state.measured_liquid_heights.values()) - - def get_last_measured_liquid_height(self, well: WellIdentifier) -> Optional[float]: + allHeights = [] # type: List[LiquidHeightInfo] + for val in self._state.measured_liquid_heights.values(): + allHeights.extend(a for a in val.values()) + return allHeights + + def get_wells_in_labware(self, labware_id: str) -> List[LiquidHeightInfo]: + """Get all well liquid heights for a particular labware.""" + return list(self._state.measured_liquid_heights[labware_id].values()) + + def get_last_measured_liquid_height( + self, labware_id: str, well_name: str + ) -> Optional[float]: """Returns the height of the liquid according to the most recent liquid level probe to this well. Returns None if no liquid probe has been done. """ try: - height = self._state.measured_liquid_heights[well].height + height = self._state.measured_liquid_heights[labware_id][well_name].height return height except KeyError: return None - def has_measured_liquid_height(self, well: WellIdentifier) -> bool: + def has_measured_liquid_height(self, labware_id: str, well_name: str) -> bool: """Returns True if the well has been liquid level probed previously.""" try: - height = self._state.measured_liquid_heights[well].height - return height is not None + self._state.measured_liquid_heights[labware_id][well_name].height + return True except KeyError: return False diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index d800f8276e9..504fa7cc2bb 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -300,17 +300,6 @@ class CurrentWell: well_name: str -class WellIdentifier(BaseModel): - """Identifying information needed for well state tracking.""" - - labware_id: str - well_name: str - - def __hash__(self) -> int: - """Needed to make WellIdentifier a key in dictionaries.""" - return hash((self.labware_id, self.well_name)) - - class LiquidHeightInfo(BaseModel): """Payload required to store recent measured liquid heights.""" diff --git a/api/tests/opentrons/protocol_engine/state/test_well_store.py b/api/tests/opentrons/protocol_engine/state/test_well_store.py index 93ab4a161bb..325021a9942 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_store.py @@ -1,7 +1,6 @@ """Well state store tests.""" import pytest from opentrons.protocol_engine.state.wells import WellStore -from opentrons.protocol_engine.types import WellIdentifier from opentrons.protocol_engine.actions.actions import SucceedCommandAction from .command_fixtures import create_liquid_probe_command @@ -15,7 +14,8 @@ def subject() -> WellStore: def test_handles_liquid_probe_success(subject: WellStore) -> None: """It should add the well to the state after a successful liquid probe.""" - well_id = WellIdentifier(labware_id="labware-id", well_name="well-name") + labware_id = "labware-id" + well_name = "well-name" liquid_probe = create_liquid_probe_command() @@ -25,4 +25,4 @@ def test_handles_liquid_probe_success(subject: WellStore) -> None: assert len(subject.state.measured_liquid_heights) == 1 - assert subject.state.measured_liquid_heights[well_id].height == 0.5 + assert subject.state.measured_liquid_heights[labware_id][well_name].height == 0.5 diff --git a/api/tests/opentrons/protocol_engine/state/test_well_view.py b/api/tests/opentrons/protocol_engine/state/test_well_view.py index 5ee70b13bda..3bd86e9dcb9 100644 --- a/api/tests/opentrons/protocol_engine/state/test_well_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_well_view.py @@ -1,6 +1,6 @@ """Well view tests.""" from datetime import datetime -from opentrons.protocol_engine.types import LiquidHeightInfo, WellIdentifier +from opentrons.protocol_engine.types import LiquidHeightInfo import pytest from opentrons.protocol_engine.state.wells import WellState, WellView @@ -8,9 +8,10 @@ @pytest.fixture def subject() -> WellView: """Get a well view test subject.""" - well_id = WellIdentifier(labware_id="labware-id", well_name="well-name") + labware_id = "labware-id" + well_name = "well-name" height_info = LiquidHeightInfo(height=0.5, last_measured=datetime.now()) - state = WellState(measured_liquid_heights={well_id: height_info}) + state = WellState(measured_liquid_heights={labware_id: {well_name: height_info}}) return WellView(state) @@ -22,21 +23,29 @@ def test_get_all(subject: WellView) -> None: def test_get_last_measured_liquid_height(subject: WellView) -> None: """Should return the height of a single well correctly for valid wells.""" - valid_details = WellIdentifier(labware_id="labware-id", well_name="well-name") + labware_id = "labware-id" + well_name = "well-name" - invalid_details = WellIdentifier( - labware_id="wrong-labware-id", well_name="wrong-well-name" + invalid_labware_id = "invalid-labware-id" + invalid_well_name = "invalid-well-name" + + assert ( + subject.get_last_measured_liquid_height(invalid_labware_id, invalid_well_name) + is None ) - assert subject.get_last_measured_liquid_height(invalid_details) is None - assert subject.get_last_measured_liquid_height(valid_details) == 0.5 + assert subject.get_last_measured_liquid_height(labware_id, well_name) == 0.5 def test_has_measured_liquid_height(subject: WellView) -> None: """Should return True for measured wells and False for ones that have no measurements.""" - valid_details = WellIdentifier(labware_id="labware-id", well_name="well-name") + labware_id = "labware-id" + well_name = "well-name" + + invalid_labware_id = "invalid-labware-id" + invalid_well_name = "invalid-well-name" - invalid_details = WellIdentifier( - labware_id="wrong-labware-id", well_name="wrong-well-name" + assert ( + subject.has_measured_liquid_height(invalid_labware_id, invalid_well_name) + is False ) - assert subject.has_measured_liquid_height(invalid_details) is False - assert subject.has_measured_liquid_height(valid_details) is True + assert subject.has_measured_liquid_height(labware_id, well_name) is True