feat(api): fix InstrumentContext.name for Flex and update LiquidClass…

….get_for() (#17289)

Closes AUTH-1295, AUTH-1299

# Overview

- fixes `InstrumentContext.name` to fetch the python API load name
of flex pipettes
- updates `LiquidClass.get_for()` to accept the a loaded instrument
object and loaded tiprack object for `pipette` and `tip_rack` args
respectively.
- changes the tiprack argument name of `get_for()` from `tiprack` to
`tip_rack` to be consistent with API naming conventions.

## Risk assessment

None

---------

Co-authored-by: Max Marrone <[email protected]>

api/release-notes-internal.md

@@ -11,6 +11,10 @@ This internal release, pulled from the `edge` branch, contains features being de
 - Python API version bumped to 2.23
 - Added liquid classes and new transfer functions
 
+### Bug Fixes In This Release (list in progress):
+- Fixed `InstrumentContext.name` so that it returns the correct API-specific names of Flex pipettes.
+
+
 ## Internal Release 2.3.0-alpha.2
 
 This internal release, pulled from the `edge` branch, contains features being developed for 8.3.0. It's for internal testing only.

api/src/opentrons/protocol_api/_liquid.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from dataclasses import dataclass
-from typing import Optional, Dict
+from typing import Optional, Dict, Union, TYPE_CHECKING
 
 from opentrons_shared_data.liquid_classes.liquid_class_definition import (
     LiquidClassSchemaV1,
@@ -12,6 +12,9 @@
     build_transfer_properties,
 )
 
+if TYPE_CHECKING:
+    from . import InstrumentContext, Labware
+
 
 @dataclass(frozen=True)
 class Liquid:
@@ -64,18 +67,42 @@ def name(self) -> str:
     def display_name(self) -> str:
         return self._display_name
 
-    def get_for(self, pipette: str, tiprack: str) -> TransferProperties:
+    def get_for(
+        self, pipette: Union[str, InstrumentContext], tip_rack: Union[str, Labware]
+    ) -> TransferProperties:
         """Get liquid class transfer properties for the specified pipette and tip."""
+        from . import InstrumentContext, Labware
+
+        if isinstance(pipette, InstrumentContext):
+            pipette_name = pipette.name
+        elif isinstance(pipette, str):
+            pipette_name = pipette
+        else:
+            raise ValueError(
+                f"{pipette} should either be an InstrumentContext object"
+                f" or a pipette name string."
+            )
+
+        if isinstance(tip_rack, Labware):
+            tiprack_uri = tip_rack.uri
+        elif isinstance(tip_rack, str):
+            tiprack_uri = tip_rack
+        else:
+            raise ValueError(
+                f"{tip_rack} should either be a tiprack Labware object"
+                f" or a tiprack URI string."
+            )
+
         try:
-            settings_for_pipette = self._by_pipette_setting[pipette]
+            settings_for_pipette = self._by_pipette_setting[pipette_name]
         except KeyError:
             raise ValueError(
-                f"No properties found for {pipette} in {self._name} liquid class"
+                f"No properties found for {pipette_name} in {self._name} liquid class"
             )
         try:
-            transfer_properties = settings_for_pipette[tiprack]
+            transfer_properties = settings_for_pipette[tiprack_uri]
         except KeyError:
             raise ValueError(
-                f"No properties found for {tiprack} in {self._name} liquid class"
+                f"No properties found for {tiprack_uri} in {self._name} liquid class"
             )
         return transfer_properties

api/src/opentrons/protocol_api/core/engine/instrument.py

@@ -42,7 +42,7 @@
 from opentrons.protocol_engine.errors.exceptions import TipNotAttachedError
 from opentrons.protocol_engine.clients import SyncClient as EngineClient
 from opentrons.protocols.api_support.definitions import MAX_SUPPORTED_VERSION
-from opentrons_shared_data.pipette.types import PipetteNameType, PIPETTE_API_NAMES_MAP
+from opentrons_shared_data.pipette.types import PIPETTE_API_NAMES_MAP
 from opentrons_shared_data.errors.exceptions import (
     UnsupportedHardwareCommand,
 )
@@ -62,6 +62,9 @@
 
 _DISPENSE_VOLUME_VALIDATION_ADDED_IN = APIVersion(2, 17)
 
+_FLEX_PIPETTE_NAMES_FIXED_IN = APIVersion(2, 23)
+"""The version after which InstrumentContext.name returns the correct API-specific names of Flex pipettes."""
+
 
 class InstrumentCore(AbstractInstrument[WellCore, LabwareCore]):
     """Instrument API core using a ProtocolEngine.
@@ -721,33 +724,29 @@ def get_pipette_name(self) -> str:
 
         Will match the load name of the actually loaded pipette,
         which may differ from the requested load name.
-        """
-        # TODO (tz, 11-23-22): revert this change when merging
-        # https://opentrons.atlassian.net/browse/RLIQ-251
-        pipette = self._engine_client.state.pipettes.get(self._pipette_id)
-        return (
-            pipette.pipetteName.value
-            if isinstance(pipette.pipetteName, PipetteNameType)
-            else pipette.pipetteName
-        )
 
-    def get_load_name(self) -> str:
-        """Get the pipette's requested API load name.
+        From API v2.15 to v2.22, this property returned an internal, engine-specific,
+         name for Flex pipettes (eg, "p50_multi_flex" instead of "flex_8channel_50").
 
-        This is the load name that is specified in the `ProtocolContext.load_instrument()`
-        method. This name might differ from the engine-specific pipette name.
+        From API v2.23 onwards, this behavior is fixed so that this property returns
+        the API-specific names of Flex pipettes.
         """
+        # TODO (tz, 11-23-22): revert this change when merging
+        # https://opentrons.atlassian.net/browse/RLIQ-251
         pipette = self._engine_client.state.pipettes.get(self._pipette_id)
-        load_name = next(
-            (
-                pip_api_name
-                for pip_api_name, pip_name in PIPETTE_API_NAMES_MAP.items()
-                if pip_name == pipette.pipetteName
-            ),
-            None,
-        )
-        assert load_name, "Load name not found."
-        return load_name
+        if self._protocol_core.api_version < _FLEX_PIPETTE_NAMES_FIXED_IN:
+            return pipette.pipetteName.value
+        else:
+            name = next(
+                (
+                    pip_api_name
+                    for pip_api_name, pip_name in PIPETTE_API_NAMES_MAP.items()
+                    if pip_name == pipette.pipetteName
+                ),
+                None,
+            )
+            assert name, "Pipette name not found."
+            return name
 
     def get_model(self) -> str:
         return self._engine_client.state.pipettes.get_model_name(self._pipette_id)
@@ -932,7 +931,7 @@ def load_liquid_class(
         """
         liquid_class_record = LiquidClassRecord(
             liquidClassName=name,
-            pipetteModel=self.get_load_name(),
+            pipetteModel=self.get_pipette_name(),
             tiprack=tiprack_uri,
             aspirate=transfer_properties.aspirate.as_shared_data_model(),
             singleDispense=transfer_properties.dispense.as_shared_data_model(),
@@ -994,8 +993,7 @@ def transfer_liquid(  # noqa: C901
             )
         tiprack_uri_for_transfer_props = tip_racks[0][1].get_uri()
         transfer_props = liquid_class.get_for(
-            pipette=self.get_load_name(),
-            tiprack=tiprack_uri_for_transfer_props,
+            pipette=self.get_pipette_name(), tip_rack=tiprack_uri_for_transfer_props
         )
         # TODO: use the ID returned by load_liquid_class in command annotations
         self.load_liquid_class(

api/src/opentrons/protocol_api/instrument_context.py

@@ -64,6 +64,7 @@
 _AIR_GAP_TRACKING_ADDED_IN = APIVersion(2, 22)
 """The version after which air gaps should be implemented with a separate call instead of an aspirate for better liquid volume tracking."""
 
+
 AdvancedLiquidHandling = v1_transfer.AdvancedLiquidHandling
 
 
@@ -2004,6 +2005,10 @@ def trash_container(
     def name(self) -> str:
         """
         The name string for the pipette (e.g., ``"p300_single"``).
+
+        From API v2.15 to v2.22, this property returned an internal name for Flex pipettes.
+        From API v2.23 onwards, this behavior is fixed so that this property returns
+        the Python Protocol API load names of Flex pipettes.
         """
         return self._core.get_pipette_name()
 

api/src/opentrons/protocols/api_support/instrument.py

@@ -99,7 +99,20 @@ def validate_tiprack(
         gen_lookup = (
             "FLEX" if ("flex" in instr_metadata or "96" in instr_metadata) else "OT2"
         )
-        valid_vols = VALID_PIP_TIPRACK_VOL[gen_lookup][instrument_name.split("_")[0]]
+
+        # TODO (spp, 2025-01-30): do what AA's note above says or at least,
+        #  fetch the 'pip_type' below from the 'model' field in pipette definitions
+        #  so that we don't have to figure it out from pipette names
+        if instrument_name.split("_")[0] == "flex":
+            # Flex's API load names have the format 'flex_1channel_1000'
+            # From API v2.23 on, this is the name returned by InstrumentContext.name
+            pip_type = "p" + instrument_name.split("_")[2]
+        else:
+            # Until API v2.23, InstrumentContext.name returned the engine-specific names
+            # of Flex pipettes. These names, as well as OT2 pipette names,
+            # have the format- 'p1000_single_gen2' or 'p1000_single_flex'
+            pip_type = instrument_name.split("_")[0]
+        valid_vols = VALID_PIP_TIPRACK_VOL[gen_lookup][pip_type]
         if tiprack_vol not in valid_vols:
             log.warning(
                 f"The pipette {instrument_name} and its tip rack {tip_rack.load_name}"

api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py

@@ -187,32 +187,60 @@ def test_pipette_id(subject: InstrumentCore) -> None:
     assert subject.pipette_id == "abc123"
 
 
-def test_get_pipette_name(
-    decoy: Decoy, mock_engine_client: EngineClient, subject: InstrumentCore
+@pytest.mark.parametrize(
+    "version",
+    [
+        APIVersion(2, 15),
+        APIVersion(2, 17),
+        APIVersion(2, 20),
+        APIVersion(2, 22),
+    ],
+)
+def test_get_pipette_name_old(
+    decoy: Decoy,
+    mock_engine_client: EngineClient,
+    mock_protocol_core: ProtocolCore,
+    subject: InstrumentCore,
+    version: APIVersion,
 ) -> None:
     """It should get the pipette's load name."""
+    decoy.when(mock_protocol_core.api_version).then_return(version)
     decoy.when(mock_engine_client.state.pipettes.get("abc123")).then_return(
         LoadedPipette.model_construct(pipetteName=PipetteNameType.P300_SINGLE)  # type: ignore[call-arg]
     )
-
-    result = subject.get_pipette_name()
-
-    assert result == "p300_single"
+    assert subject.get_pipette_name() == "p300_single"
+    decoy.when(mock_engine_client.state.pipettes.get("abc123")).then_return(
+        LoadedPipette.model_construct(pipetteName=PipetteNameType.P1000_96)  # type: ignore[call-arg]
+    )
+    assert subject.get_pipette_name() == "p1000_96"
+    decoy.when(mock_engine_client.state.pipettes.get("abc123")).then_return(
+        LoadedPipette.model_construct(pipetteName=PipetteNameType.P50_SINGLE_FLEX)  # type: ignore[call-arg]
+    )
+    assert subject.get_pipette_name() == "p50_single_flex"
 
 
-def test_get_pipette_load_name(
-    decoy: Decoy, mock_engine_client: EngineClient, subject: InstrumentCore
+@pytest.mark.parametrize("version", versions_at_or_above(APIVersion(2, 23)))
+def test_get_pipette_name_new(
+    decoy: Decoy,
+    mock_engine_client: EngineClient,
+    mock_protocol_core: ProtocolCore,
+    subject: InstrumentCore,
+    version: APIVersion,
 ) -> None:
     """It should get the pipette's API-specific load name."""
+    decoy.when(mock_protocol_core.api_version).then_return(version)
     decoy.when(mock_engine_client.state.pipettes.get("abc123")).then_return(
         LoadedPipette.model_construct(pipetteName=PipetteNameType.P300_SINGLE)  # type: ignore[call-arg]
     )
-    assert subject.get_load_name() == "p300_single"
-
+    assert subject.get_pipette_name() == "p300_single"
     decoy.when(mock_engine_client.state.pipettes.get("abc123")).then_return(
         LoadedPipette.model_construct(pipetteName=PipetteNameType.P1000_96)  # type: ignore[call-arg]
     )
-    assert subject.get_load_name() == "flex_96channel_1000"
+    assert subject.get_pipette_name() == "flex_96channel_1000"
+    decoy.when(mock_engine_client.state.pipettes.get("abc123")).then_return(
+        LoadedPipette.model_construct(pipetteName=PipetteNameType.P50_SINGLE_FLEX)  # type: ignore[call-arg]
+    )
+    assert subject.get_pipette_name() == "flex_1channel_50"
 
 
 def test_get_mount(
@@ -1671,11 +1699,14 @@ def test_liquid_probe_with_recovery(
     )
 
 
+@pytest.mark.parametrize("version", versions_at_or_above(APIVersion(2, 23)))
 def test_load_liquid_class(
     decoy: Decoy,
     mock_engine_client: EngineClient,
+    mock_protocol_core: ProtocolCore,
     subject: InstrumentCore,
     minimal_liquid_class_def2: LiquidClassSchemaV1,
+    version: APIVersion,
 ) -> None:
     """It should send the load liquid class command to the engine."""
     sample_aspirate_data = minimal_liquid_class_def2.byPipette[0].byTipType[0].aspirate
@@ -1686,6 +1717,7 @@ def test_load_liquid_class(
         minimal_liquid_class_def2.byPipette[0].byTipType[0].multiDispense
     )
 
+    decoy.when(mock_protocol_core.api_version).then_return(version)
     test_liq_class = decoy.mock(cls=LiquidClass)
     test_transfer_props = decoy.mock(cls=TransferProperties)
 

api/tests/opentrons/protocol_api/core/engine/test_transfer_components_executor.py

@@ -36,7 +36,7 @@ def sample_transfer_props(
 ) -> TransferProperties:
     """Return a mocked out liquid class fixture."""
     return LiquidClass.create(maximal_liquid_class_def).get_for(
-        pipette="flex_1channel_50", tiprack="opentrons_flex_96_tiprack_50ul"
+        pipette="flex_1channel_50", tip_rack="opentrons_flex_96_tiprack_50ul"
     )
 
 

api/tests/opentrons/protocol_api/test_liquid_class.py

@@ -1,10 +1,12 @@
 """Tests for LiquidClass methods."""
 import pytest
+from decoy import Decoy
 
 from opentrons_shared_data.liquid_classes.liquid_class_definition import (
     LiquidClassSchemaV1,
 )
 from opentrons.protocol_api import LiquidClass
+from opentrons.protocol_api import InstrumentContext, Labware
 
 
 def test_create_liquid_class(
@@ -17,6 +19,7 @@ def test_create_liquid_class(
 
 
 def test_get_for_pipette_and_tip(
+    decoy: Decoy,
     minimal_liquid_class_def2: LiquidClassSchemaV1,
 ) -> None:
     """It should get the properties for the specified pipette and tip."""
@@ -26,6 +29,15 @@ def test_get_for_pipette_and_tip(
         10.0: 40.0,
         20.0: 30.0,
     }
+    mock_instrument = decoy.mock(cls=InstrumentContext)
+    mock_tiprack = decoy.mock(cls=Labware)
+    decoy.when(mock_instrument.name).then_return("flex_1channel_50")
+    decoy.when(mock_tiprack.uri).then_return("opentrons_flex_96_tiprack_50ul")
+    result_2 = liq_class.get_for(mock_instrument, mock_tiprack)
+    assert result_2.aspirate.flow_rate_by_volume.as_dict() == {
+        10.0: 40.0,
+        20.0: 30.0,
+    }
 
 
 def test_get_for_raises_for_incorrect_pipette_or_tip(

api/tests/opentrons/protocol_api/test_protocol_context.py

@@ -283,6 +283,7 @@ def test_load_instrument(
     ).then_return(mock_instrument_core)
 
     decoy.when(mock_instrument_core.get_pipette_name()).then_return("Gandalf the Grey")
+    decoy.when(mock_instrument_core.get_model()).then_return("wizard")
     decoy.when(mock_core.get_disposal_locations()).then_raise(
         NoTrashDefinedError("No trash!")
     )

api/tests/opentrons/protocol_api_integration/test_liquid_classes.py

@@ -13,7 +13,7 @@ def test_liquid_class_creation_and_property_fetching(
 ) -> None:
     """It should create the liquid class and provide access to its properties."""
     pipette_load_name = "flex_8channel_50"
-    simulated_protocol_context.load_instrument(pipette_load_name, mount="left")
+    p50 = simulated_protocol_context.load_instrument(pipette_load_name, mount="left")
     tiprack = simulated_protocol_context.load_labware(
         "opentrons_flex_96_tiprack_50ul", "D1"
     )
@@ -24,10 +24,7 @@ def test_liquid_class_creation_and_property_fetching(
 
     # TODO (spp, 2024-10-17): update this to fetch pipette load name from instrument context
     assert (
-        water.get_for(
-            pipette_load_name, tiprack.uri
-        ).dispense.flow_rate_by_volume.get_for_volume(1)
-        == 50
+        water.get_for(p50, tiprack).dispense.flow_rate_by_volume.get_for_volume(1) == 50
     )
     assert water.get_for(pipette_load_name, tiprack.uri).aspirate.submerge.speed == 100