fix(api): Fully stringify well names (#13764)

Co-authored-by: Jeremy Leon <[email protected]>

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

@@ -0,0 +1,56 @@
+from opentrons.protocol_engine.clients.sync_client import SyncClient
+from opentrons.protocol_engine.types import (
+    DeckSlotLocation,
+    LabwareLocation,
+    ModuleLocation,
+    OnLabwareLocation,
+)
+
+
+def well(engine_client: SyncClient, well_name: str, labware_id: str) -> str:
+    """Return a human-readable string representing a well and its location.
+
+    For example: "A1 of My Cool Labware on C2".
+    """
+    labware_location = OnLabwareLocation(labwareId=labware_id)
+    return f"{well_name} of {_labware_location_string(engine_client, labware_location)}"
+
+
+def _labware_location_string(
+    engine_client: SyncClient, location: LabwareLocation
+) -> str:
+    if isinstance(location, DeckSlotLocation):
+        # TODO(mm, 2023-10-11):
+        # Ideally, we might want to use the display name specified by the deck definition?
+        return f"slot {location.slotName.id}"
+
+    elif isinstance(location, ModuleLocation):
+        module_name = engine_client.state.modules.get_definition(
+            module_id=location.moduleId
+        ).displayName
+        module_on = engine_client.state.modules.get_location(
+            module_id=location.moduleId
+        )
+        module_on_string = _labware_location_string(engine_client, module_on)
+        return f"{module_name} on {module_on_string}"
+
+    elif isinstance(location, OnLabwareLocation):
+        labware_name = _labware_name(engine_client, location.labwareId)
+        labware_on = engine_client.state.labware.get_location(
+            labware_id=location.labwareId
+        )
+        labware_on_string = _labware_location_string(engine_client, labware_on)
+        return f"{labware_name} on {labware_on_string}"
+
+    elif location == "offDeck":
+        return "[off-deck]"
+
+
+def _labware_name(engine_client: SyncClient, labware_id: str) -> str:
+    """Return the user-specified labware label, or fall back to the display name from the def."""
+    user_name = engine_client.state.labware.get_display_name(labware_id=labware_id)
+    definition_name = engine_client.state.labware.get_definition(
+        labware_id=labware_id
+    ).metadata.displayName
+
+    return user_name if user_name is not None else definition_name

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

@@ -9,6 +9,7 @@
 from opentrons.types import Point
 
 from . import point_calculations
+from . import stringify
 from ..well import AbstractWellCore
 from ..._liquid import Liquid
 
@@ -72,9 +73,12 @@ def set_has_tip(self, value: bool) -> None:
         )
 
     def get_display_name(self) -> str:
-        """Get the well's full display name."""
-        parent = self._engine_client.state.labware.get_display_name(self._labware_id)
-        return f"{self._name} of {parent}"
+        """Get the full display name of the well (e.g. "A1 of Some Labware on 5")."""
+        return stringify.well(
+            engine_client=self._engine_client,
+            well_name=self._name,
+            labware_id=self._labware_id,
+        )
 
     def get_name(self) -> str:
         """Get the name of the well (e.g. "A1")."""

api/src/opentrons/protocol_api/core/legacy/legacy_well_core.py

@@ -75,7 +75,7 @@ def set_has_tip(self, value: bool) -> None:
         self._has_tip = value
 
     def get_display_name(self) -> str:
-        """Get the well's full display name."""
+        """Get the full display name of the well (e.g. "A1 of Some Labware on 5")."""
         return self._display_name
 
     def get_name(self) -> str:

api/src/opentrons/protocol_api/core/well.py

@@ -41,7 +41,7 @@ def set_has_tip(self, value: bool) -> None:
 
     @abstractmethod
     def get_display_name(self) -> str:
-        """Get the full display name of the well (e.g. "A1 of Some Labware")."""
+        """Get the full display name of the well (e.g. "A1 of Some Labware on 5")."""
 
     @abstractmethod
     def get_name(self) -> str:

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

@@ -0,0 +1,108 @@
+"""Unit tests for `stringify`."""
+
+
+from decoy import Decoy
+from opentrons_shared_data.labware.labware_definition import LabwareDefinition
+
+from opentrons.protocol_api.core.engine import stringify as subject
+from opentrons.protocol_engine.clients.sync_client import SyncClient
+from opentrons.protocol_engine.types import (
+    OFF_DECK_LOCATION,
+    DeckSlotLocation,
+    ModuleDefinition,
+    ModuleLocation,
+    OnLabwareLocation,
+)
+from opentrons.types import DeckSlotName
+
+
+def _make_dummy_labware_definition(
+    decoy: Decoy, display_name: str
+) -> LabwareDefinition:
+    mock = decoy.mock(cls=LabwareDefinition)
+    decoy.when(mock.metadata.displayName).then_return(display_name)
+    return mock
+
+
+def _make_dummy_module_definition(decoy: Decoy, display_name: str) -> ModuleDefinition:
+    mock = decoy.mock(cls=ModuleDefinition)
+    decoy.when(mock.displayName).then_return(display_name)
+    return mock
+
+
+def test_well_on_labware_without_user_display_name(decoy: Decoy) -> None:
+    """Test stringifying a well on a labware that doesn't have a user-defined label."""
+    mock_client = decoy.mock(cls=SyncClient)
+    decoy.when(mock_client.state.labware.get_display_name("labware-id")).then_return(
+        None
+    )
+    decoy.when(mock_client.state.labware.get_definition("labware-id")).then_return(
+        _make_dummy_labware_definition(decoy, "definition-display-name")
+    )
+    decoy.when(mock_client.state.labware.get_location("labware-id")).then_return(
+        OFF_DECK_LOCATION
+    )
+
+    result = subject.well(
+        engine_client=mock_client, well_name="well-name", labware_id="labware-id"
+    )
+    assert result == "well-name of definition-display-name on [off-deck]"
+
+
+def test_well_on_labware_with_user_display_name(decoy: Decoy) -> None:
+    """Test stringifying a well on a labware that does have a user-defined label."""
+    mock_client = decoy.mock(cls=SyncClient)
+    decoy.when(mock_client.state.labware.get_display_name("labware-id")).then_return(
+        "user-display-name"
+    )
+    decoy.when(mock_client.state.labware.get_definition("labware-id")).then_return(
+        _make_dummy_labware_definition(decoy, "definition-display-name")
+    )
+    decoy.when(mock_client.state.labware.get_location("labware-id")).then_return(
+        OFF_DECK_LOCATION
+    )
+
+    result = subject.well(
+        engine_client=mock_client, well_name="well-name", labware_id="labware-id"
+    )
+    assert result == "well-name of user-display-name on [off-deck]"
+
+
+def test_well_on_labware_with_complicated_location(decoy: Decoy) -> None:
+    """Test stringifying a well on a labware with a deeply-nested location."""
+    mock_client = decoy.mock(cls=SyncClient)
+
+    decoy.when(mock_client.state.labware.get_display_name("labware-id-1")).then_return(
+        None
+    )
+    decoy.when(mock_client.state.labware.get_definition("labware-id-1")).then_return(
+        _make_dummy_labware_definition(decoy, "lw-1-display-name")
+    )
+    decoy.when(mock_client.state.labware.get_location("labware-id-1")).then_return(
+        OnLabwareLocation(labwareId="labware-id-2")
+    )
+
+    decoy.when(mock_client.state.labware.get_display_name("labware-id-2")).then_return(
+        None
+    )
+    decoy.when(mock_client.state.labware.get_definition("labware-id-2")).then_return(
+        _make_dummy_labware_definition(decoy, "lw-2-display-name")
+    )
+    decoy.when(mock_client.state.labware.get_location("labware-id-2")).then_return(
+        ModuleLocation(moduleId="module-id")
+    )
+
+    decoy.when(mock_client.state.modules.get_definition("module-id")).then_return(
+        _make_dummy_module_definition(decoy, "module-display-name")
+    )
+    decoy.when(mock_client.state.modules.get_location("module-id")).then_return(
+        DeckSlotLocation(slotName=DeckSlotName.SLOT_C2)
+    )
+
+    result = subject.well(
+        engine_client=mock_client, well_name="well-name", labware_id="labware-id-1"
+    )
+    assert (
+        result
+        == "well-name of lw-1-display-name on lw-2-display-name on module-display-name on slot C2"
+    )

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

@@ -14,7 +14,7 @@
 from opentrons.types import Point
 
 from opentrons.protocol_api._liquid import Liquid
-from opentrons.protocol_api.core.engine import WellCore, point_calculations
+from opentrons.protocol_api.core.engine import WellCore, point_calculations, stringify
 
 
 @pytest.fixture(autouse=True)
@@ -26,6 +26,13 @@ def patch_mock_point_calculations(
         monkeypatch.setattr(point_calculations, name, decoy.mock(func=func))
 
 
+@pytest.fixture(autouse=True)
+def patch_mock_stringify(decoy: Decoy, monkeypatch: pytest.MonkeyPatch) -> None:
+    """Mock out stringify.py functions."""
+    for name, func in inspect.getmembers(stringify, inspect.isfunction):
+        monkeypatch.setattr(stringify, name, decoy.mock(func=func))
+
+
 @pytest.fixture
 def mock_engine_client(decoy: Decoy) -> EngineClient:
     """Get a mock ProtocolEngine synchronous client."""
@@ -73,10 +80,14 @@ def test_display_name(
 ) -> None:
     """It should have a display name."""
     decoy.when(
-        mock_engine_client.state.labware.get_display_name("labware-id")
-    ).then_return("Cool Labware")
+        stringify.well(
+            engine_client=mock_engine_client,
+            well_name="well-name",
+            labware_id="labware-id",
+        )
+    ).then_return("Matthew Zwimpfer")
 
-    assert subject.get_display_name() == "well-name of Cool Labware"
+    assert subject.get_display_name() == "Matthew Zwimpfer"
 
 
 @pytest.mark.parametrize(

api/tests/opentrons/test_execute.py

@@ -72,14 +72,12 @@ async def dummy_delay(self: Any, duration_s: float) -> None:
             ],
         ),
         (
-            # FIXME(2023-10-04): This run log is wrong. It should match the one above.
-            # https://opentrons.atlassian.net/browse/RSS-368
             "testosaur_v2_14.py",
             [
-                "Picking up tip from A1 of None",
-                "Aspirating 100.0 uL from A1 of None at 500.0 uL/sec",
-                "Dispensing 100.0 uL into B1 of None at 1000.0 uL/sec",
-                "Dropping tip into H12 of None",
+                "Picking up tip from A1 of Opentrons 96 Tip Rack 1000 µL on slot 1",
+                "Aspirating 100.0 uL from A1 of Corning 96 Well Plate 360 µL Flat on slot 2 at 500.0 uL/sec",
+                "Dispensing 100.0 uL into B1 of Corning 96 Well Plate 360 µL Flat on slot 2 at 1000.0 uL/sec",
+                "Dropping tip into H12 of Opentrons 96 Tip Rack 1000 µL on slot 1",
             ],
         ),
     ],

api/tests/opentrons/test_simulate.py

@@ -82,13 +82,11 @@ def test_simulate_without_filename(protocol: Protocol, protocol_file: str) -> No
         ),
         (
             "testosaur_v2_14.py",
-            # FIXME(2023-10-04): This run log is wrong. It should match the one above.
-            # https://opentrons.atlassian.net/browse/RSS-368
             [
-                "Picking up tip from A1 of None",
-                "Aspirating 100.0 uL from A1 of None at 500.0 uL/sec",
-                "Dispensing 100.0 uL into B1 of None at 1000.0 uL/sec",
-                "Dropping tip into H12 of None",
+                "Picking up tip from A1 of Opentrons 96 Tip Rack 1000 µL on slot 1",
+                "Aspirating 100.0 uL from A1 of Corning 96 Well Plate 360 µL Flat on slot 2 at 500.0 uL/sec",
+                "Dispensing 100.0 uL into B1 of Corning 96 Well Plate 360 µL Flat on slot 2 at 1000.0 uL/sec",
+                "Dropping tip into H12 of Opentrons 96 Tip Rack 1000 µL on slot 1",
             ],
         ),
     ],