Skip to content

Commit

Permalink
fix display name in deck conflict error message, add separate display…
Browse files Browse the repository at this point in the history
… anme getter
  • Loading branch information
sanni-t committed Dec 12, 2023
1 parent a859387 commit 02ab742
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 20 deletions.
4 changes: 3 additions & 1 deletion api/src/opentrons/protocol_api/core/engine/labware.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ def __init__(self, labware_id: str, engine_client: ProtocolEngineClient) -> None

labware_state = engine_client.state.labware
self._definition = labware_state.get_definition(labware_id)
self._user_display_name = labware_state.get_display_name(labware_id)
self._user_display_name = labware_state.get_user_specified_display_name(
labware_id
)

@property
def labware_id(self) -> str:
Expand Down
4 changes: 3 additions & 1 deletion api/src/opentrons/protocol_api/core/engine/stringify.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ def _labware_location_string(

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)
user_name = engine_client.state.labware.get_user_specified_display_name(
labware_id=labware_id
)
definition_name = engine_client.state.labware.get_definition(
labware_id=labware_id
).metadata.displayName
Expand Down
13 changes: 12 additions & 1 deletion api/src/opentrons/protocol_engine/state/labware.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,21 @@ def get_definition(self, labware_id: str) -> LabwareDefinition:
LabwareUri(self.get(labware_id).definitionUri)
)

def get_display_name(self, labware_id: str) -> Optional[str]:
def get_user_specified_display_name(self, labware_id: str) -> Optional[str]:
"""Get the labware's user-specified display name, if set."""
return self.get(labware_id).displayName

def get_display_name(self, labware_id: str) -> str:
"""Get the labware's display name.
If a user-specified display name exists, will return that, else will return
display name from the definition.
"""
return (
self.get_user_specified_display_name(labware_id)
or self.get_definition(labware_id).metadata.displayName
)

def get_deck_definition(self) -> DeckDefinitionV4:
"""Get the current deck definition."""
return self._state.deck_definition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def test_get_definition(subject: LabwareCore) -> None:
def test_get_user_display_name(decoy: Decoy, mock_engine_client: EngineClient) -> None:
"""It should get the labware's user-provided label, if any."""
decoy.when(
mock_engine_client.state.labware.get_display_name("cool-labware")
mock_engine_client.state.labware.get_user_specified_display_name("cool-labware")
).then_return("Cool Label")

subject = LabwareCore(labware_id="cool-labware", engine_client=mock_engine_client)
Expand Down Expand Up @@ -149,7 +149,7 @@ def test_get_name_load_name(subject: LabwareCore) -> None:
def test_get_name_display_name(decoy: Decoy, mock_engine_client: EngineClient) -> None:
"""It should get the user display name when one is defined."""
decoy.when(
mock_engine_client.state.labware.get_display_name("cool-labware")
mock_engine_client.state.labware.get_user_specified_display_name("cool-labware")
).then_return("my cool display name")

subject = LabwareCore(labware_id="cool-labware", engine_client=mock_engine_client)
Expand Down
24 changes: 12 additions & 12 deletions api/tests/opentrons/protocol_api/core/engine/test_stringify.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ def _make_dummy_module_definition(decoy: Decoy, display_name: str) -> ModuleDefi
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_user_specified_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")
)
Expand All @@ -52,9 +52,9 @@ def test_well_on_labware_without_user_display_name(decoy: Decoy) -> None:
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_user_specified_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")
)
Expand All @@ -72,19 +72,19 @@ 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_user_specified_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_user_specified_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")
)
Expand Down
36 changes: 33 additions & 3 deletions api/tests/opentrons/protocol_engine/state/test_labware_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ def test_find_applicable_labware_offset() -> None:
)


def test_get_display_name() -> None:
def test_get_user_specified_display_name() -> None:
"""It should get a labware's user-specified display name."""
subject = get_labware_view(
labware_by_id={
Expand All @@ -994,8 +994,38 @@ def test_get_display_name() -> None:
},
)

assert subject.get_display_name("plate_with_display_name") == "Fancy Plate Name"
assert subject.get_display_name("reservoir_without_display_name") is None
assert (
subject.get_user_specified_display_name("plate_with_display_name")
== "Fancy Plate Name"
)
assert (
subject.get_user_specified_display_name("reservoir_without_display_name")
is None
)


def test_get_display_name(
well_plate_def: LabwareDefinition,
reservoir_def: LabwareDefinition,
) -> None:
"""It should get the labware's display name."""
subject = get_labware_view(
labware_by_id={
"plate_with_custom_display_name": plate,
"reservoir_with_default_display_name": reservoir,
},
definitions_by_uri={
"some-plate-uri": well_plate_def,
"some-reservoir-uri": reservoir_def,
},
)
assert (
subject.get_display_name("plate_with_custom_display_name") == "Fancy Plate Name"
)
assert (
subject.get_display_name("reservoir_with_default_display_name")
== "NEST 12 Well Reservoir 15 mL"
)


def test_get_fixed_trash_id() -> None:
Expand Down

0 comments on commit 02ab742

Please sign in to comment.