From ae90548fff5ca8db59079871dc093e367eaa975d Mon Sep 17 00:00:00 2001 From: CaseyBatten Date: Tue, 24 Dec 2024 16:10:10 -0500 Subject: [PATCH] test fixture corrections and relocation of labware with updated schema to v3 dir --- .../protocol_api/protocol_context.py | 2 +- .../protocol_engine/commands/load_labware.py | 14 ++++++++++++- .../protocol_engine/execution/equipment.py | 21 ++++--------------- api/tests/opentrons/conftest.py | 1 + .../core/engine/test_protocol_core.py | 12 +++++------ .../state/test_labware_view_old.py | 4 ++-- .../1.json | 2 +- .../protocol_engine_lid_stack_object/1.json | 2 +- shared-data/labware/schemas/3.json | 3 ++- .../labware/labware_definition.py | 12 +++++------ 10 files changed, 37 insertions(+), 36 deletions(-) rename shared-data/labware/definitions/{2 => 3}/opentrons_tough_pcr_auto_sealing_lid/1.json (99%) rename shared-data/labware/definitions/{2 => 3}/protocol_engine_lid_stack_object/1.json (97%) diff --git a/api/src/opentrons/protocol_api/protocol_context.py b/api/src/opentrons/protocol_api/protocol_context.py index 2bb5645e1a5..b9f96e4d536 100644 --- a/api/src/opentrons/protocol_api/protocol_context.py +++ b/api/src/opentrons/protocol_api/protocol_context.py @@ -444,7 +444,7 @@ def load_labware( values as the ``load_name`` parameter of :py:meth:`.load_adapter`. The adapter will use the same namespace as the labware, and the API will choose the adapter's version automatically. - :param lid: An lid to load the on top of the main labware. Accepts the same + :param lid: A lid to load the on top of the main labware. Accepts the same values as the ``load_name`` parameter of :py:meth:`.load_lid_stack`. The lid will use the same namespace as the labware, and the API will choose the lid's version automatically. diff --git a/api/src/opentrons/protocol_engine/commands/load_labware.py b/api/src/opentrons/protocol_engine/commands/load_labware.py index 6b65fe239e4..d0e83863616 100644 --- a/api/src/opentrons/protocol_engine/commands/load_labware.py +++ b/api/src/opentrons/protocol_engine/commands/load_labware.py @@ -172,6 +172,19 @@ async def execute( top_labware_definition=loaded_labware.definition, bottom_labware_id=verified_location.labwareId, ) + # Validate load location is valid for lids + if ( + labware_validation.validate_definition_is_lid( + definition=loaded_labware.definition + ) + and loaded_labware.definition.compatibleParentLabware is not None + and self._state_view.labware.get_load_name(verified_location.labwareId) + not in loaded_labware.definition.compatibleParentLabware + ): + raise ValueError( + f"Labware Lid {params.loadName} may not be loaded on parent labware {self._state_view.labware.get_display_name(verified_location.labwareId)}." + ) + # Validate labware for the absorbance reader elif isinstance(params.location, ModuleLocation): module = self._state_view.modules.get(params.location.moduleId) @@ -179,7 +192,6 @@ async def execute( self._state_view.labware.raise_if_labware_incompatible_with_plate_reader( loaded_labware.definition ) - return SuccessData( public=LoadLabwareResult( labwareId=loaded_labware.labware_id, diff --git a/api/src/opentrons/protocol_engine/execution/equipment.py b/api/src/opentrons/protocol_engine/execution/equipment.py index 9cb1fc77828..3d26b355741 100644 --- a/api/src/opentrons/protocol_engine/execution/equipment.py +++ b/api/src/opentrons/protocol_engine/execution/equipment.py @@ -3,7 +3,6 @@ from typing import Optional, overload, Union, List from opentrons_shared_data.pipette.types import PipetteNameType -from opentrons_shared_data.labware.labware_definition import LabwareRole from opentrons.calibration_storage.helpers import uri_from_details from opentrons.protocols.models import LabwareDefinition @@ -169,20 +168,6 @@ async def load_labware( version=version, ) - # Validate definition allows for load conditions - if LabwareRole.lid in definition.allowedRoles: - if isinstance(location, OnLabwareLocation): - parent_labware = self._state_store.labware.get_load_name( - location.labwareId - ) - if ( - definition.compatibleParentLabware is not None - and parent_labware not in definition.compatibleParentLabware - ): - raise ValueError( - f"Labware Lid {load_name} may not be loaded on parent labware {self._state_store.labware.get_display_name(location.labwareId)}." - ) - labware_id = ( labware_id if labware_id is not None else self._model_utils.generate_id() ) @@ -434,14 +419,16 @@ async def load_lids( version=version, ) - if quantity > definition.stackLimit: + stack_limit = definition.stackLimit if definition.stackLimit is not None else 1 + if quantity > stack_limit: raise ValueError( - f"Requested quantity {quantity} is greater than the stack limit of {definition.stackLimit} provided by definition for {load_name}." + f"Requested quantity {quantity} is greater than the stack limit of {stack_limit} provided by definition for {load_name}." ) # Allow propagation of ModuleNotLoadedError. if ( isinstance(location, DeckSlotLocation) + and definition.parameters.isDeckSlotCompatible is not None and not definition.parameters.isDeckSlotCompatible ): raise ValueError( diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py index 7be480cfe0b..20b8d3a4502 100755 --- a/api/tests/opentrons/conftest.py +++ b/api/tests/opentrons/conftest.py @@ -608,6 +608,7 @@ def minimal_labware_def() -> LabwareDefinition: "displayCategory": "other", "displayVolumeUnits": "mL", }, + "allowedRoles": ["labware"], "cornerOffsetFromSlot": {"x": 10, "y": 10, "z": 5}, "parameters": { "isTiprack": False, diff --git a/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py b/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py index cfa3773f6a7..2889a47cea9 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py @@ -789,12 +789,12 @@ def test_load_lid( ).then_return( commands.LoadLidResult( labwareId="abc123", - definition=LabwareDefinition.construct(), # type: ignore[call-arg] + definition=LabwareDefinition.model_construct(ordering=[]), # type: ignore[call-arg] ) ) decoy.when(mock_engine_client.state.labware.get_definition("abc123")).then_return( - LabwareDefinition.construct(ordering=[]) # type: ignore[call-arg] + LabwareDefinition.model_construct(ordering=[]) # type: ignore[call-arg] ) result = subject.load_lid( @@ -823,7 +823,7 @@ def test_load_lid( slot_name=DeckSlotName.SLOT_5, ) ).then_return( - LoadedLabware.construct(id="abc123") # type: ignore[call-arg] + LoadedLabware.model_construct(id="abc123") # type: ignore[call-arg] ) assert subject.get_slot_item(DeckSlotName.SLOT_5) is result @@ -862,13 +862,13 @@ def test_load_lid_stack( commands.LoadLidStackResult( stackLabwareId="abc123", labwareIds=["1", "2", "3", "4", "5"], - definition=LabwareDefinition.construct(), # type: ignore[call-arg] + definition=LabwareDefinition.model_construct(), # type: ignore[call-arg] location=DeckSlotLocation(slotName=DeckSlotName.SLOT_5), ) ) decoy.when(mock_engine_client.state.labware.get_definition("abc123")).then_return( - LabwareDefinition.construct(ordering=[]) # type: ignore[call-arg] + LabwareDefinition.model_construct(ordering=[]) # type: ignore[call-arg] ) result = subject.load_lid_stack( @@ -898,7 +898,7 @@ def test_load_lid_stack( slot_name=DeckSlotName.SLOT_5, ) ).then_return( - LoadedLabware.construct(id="abc123") # type: ignore[call-arg] + LoadedLabware.model_construct(id="abc123") # type: ignore[call-arg] ) assert subject.get_slot_item(DeckSlotName.SLOT_5) is result diff --git a/api/tests/opentrons/protocol_engine/state/test_labware_view_old.py b/api/tests/opentrons/protocol_engine/state/test_labware_view_old.py index 52a5609f3b6..ac92d5e5eaf 100644 --- a/api/tests/opentrons/protocol_engine/state/test_labware_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_labware_view_old.py @@ -1414,7 +1414,7 @@ def test_raise_if_labware_cannot_be_stacked_on_labware_on_adapter() -> None: @pytest.mark.parametrize( argnames=[ "allowed_roles", - "stacking_quirks", + "stack_limit", "exception", ], argvalues=[ @@ -1489,9 +1489,9 @@ def test_labware_stacking_height_passes_or_raises( stackingOffsetWithLabware={ "test": SharedDataOverlapOffset(x=0, y=0, z=0) }, + stackLimit=stack_limit, ), bottom_labware_id="labware-id4", - stackLimit=stack_limit, ) diff --git a/shared-data/labware/definitions/2/opentrons_tough_pcr_auto_sealing_lid/1.json b/shared-data/labware/definitions/3/opentrons_tough_pcr_auto_sealing_lid/1.json similarity index 99% rename from shared-data/labware/definitions/2/opentrons_tough_pcr_auto_sealing_lid/1.json rename to shared-data/labware/definitions/3/opentrons_tough_pcr_auto_sealing_lid/1.json index 18b587850f4..479fd00b772 100644 --- a/shared-data/labware/definitions/2/opentrons_tough_pcr_auto_sealing_lid/1.json +++ b/shared-data/labware/definitions/3/opentrons_tough_pcr_auto_sealing_lid/1.json @@ -37,7 +37,7 @@ }, "namespace": "opentrons", "version": 1, - "schemaVersion": 2, + "schemaVersion": 3, "stackingOffsetWithModule": { "thermocyclerModuleV2": { "x": 0, diff --git a/shared-data/labware/definitions/2/protocol_engine_lid_stack_object/1.json b/shared-data/labware/definitions/3/protocol_engine_lid_stack_object/1.json similarity index 97% rename from shared-data/labware/definitions/2/protocol_engine_lid_stack_object/1.json rename to shared-data/labware/definitions/3/protocol_engine_lid_stack_object/1.json index 84cbb3784db..f8aeb02350c 100644 --- a/shared-data/labware/definitions/2/protocol_engine_lid_stack_object/1.json +++ b/shared-data/labware/definitions/3/protocol_engine_lid_stack_object/1.json @@ -37,5 +37,5 @@ }, "namespace": "opentrons", "version": 1, - "schemaVersion": 2 + "schemaVersion": 3 } diff --git a/shared-data/labware/schemas/3.json b/shared-data/labware/schemas/3.json index a663f063a6a..d386cbfbb8d 100644 --- a/shared-data/labware/schemas/3.json +++ b/shared-data/labware/schemas/3.json @@ -594,7 +594,8 @@ }, "stackLimit": { "type": "number", - "description": "The limit representing the maximum stack size for a given labware." + "description": "The limit representing the maximum stack size for a given labware.", + "additionalProperties": false }, "compatibleParentLabware": { "type": "array", diff --git a/shared-data/python/opentrons_shared_data/labware/labware_definition.py b/shared-data/python/opentrons_shared_data/labware/labware_definition.py index 0be210efa81..c25f37962c1 100644 --- a/shared-data/python/opentrons_shared_data/labware/labware_definition.py +++ b/shared-data/python/opentrons_shared_data/labware/labware_definition.py @@ -182,10 +182,10 @@ class Parameters(BaseModel): magneticModuleEngageHeight: Optional[_NonNegativeNumber] = Field( None, description="Distance to move magnetic module magnets to engage" ) - isDeckSlotCompatible: bool = Field( - True, - description="Flag marking whether a labware is compatible by with" - " being placed or loaded in a base deck slot, defaults to true.", + isDeckSlotCompatible: Optional[bool] = Field( + None, + description="Flag marking whether a labware is compatible with placement" + " or load into a base deck slot, will be treated as true if unspecified.", ) @@ -737,8 +737,8 @@ class LabwareDefinition(BaseModel): None, description="A dictionary holding all unique inner well geometries in a labware.", ) - stackLimit: int = Field( - 1, + stackLimit: Optional[int] = Field( + None, description="The limit representing the maximum stack size for a given labware," " defaults to 1 when unspecified indicating a single labware with no labware below it.", )