From cf5d62e5f6a4cf75b569df4c0bbf87fd19968f98 Mon Sep 17 00:00:00 2001 From: Sanniti Date: Thu, 18 Jul 2024 18:06:33 -0400 Subject: [PATCH] revert csv file type to be nested type, add file name to it --- .../protocol_api/_parameter_context.py | 5 +++-- api/src/opentrons/protocol_engine/types.py | 12 ++++++++++- .../parameters/csv_parameter_definition.py | 15 ++++++------- .../test_csv_parameter_definition.py | 10 ++++----- .../robot_server/protocols/analysis_store.py | 5 +++-- ...lyses_with_csv_file_parameters.tavern.yaml | 4 +++- .../tests/protocols/test_analysis_store.py | 21 +++++++++++++------ .../tests/protocols/test_protocols_router.py | 5 ++++- robot-server/tests/runs/test_run_store.py | 1 - 9 files changed, 52 insertions(+), 26 deletions(-) diff --git a/api/src/opentrons/protocol_api/_parameter_context.py b/api/src/opentrons/protocol_api/_parameter_context.py index 7a5977ffacf..52e423056af 100644 --- a/api/src/opentrons/protocol_api/_parameter_context.py +++ b/api/src/opentrons/protocol_api/_parameter_context.py @@ -20,6 +20,7 @@ RunTimeParameter, PrimitiveRunTimeParamValuesType, CsvRunTimeParamFilesType, + FileInfo, ) from ._parameters import Parameters @@ -240,8 +241,8 @@ def initialize_csv_files( f" but '{variable_name}' is not a CSV parameter." ) - parameter.id = file_id - # TODO (spp, 2024-07-16): assign the file as parameter.value + parameter.file_info = FileInfo(id=file_id, name="") + # TODO (spp, 2024-07-16): set the file name and assign the file as parameter.value. # Most likely, we will be creating a temporary file copy of the original # to pass onto the protocol context diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index ccff3758368..b83c8228ca7 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -1039,13 +1039,23 @@ class EnumParameter(RTPBase): ) +class FileInfo(BaseModel): + """A file UUID descriptor.""" + + id: str = Field( + ..., + description="The UUID identifier of the file stored on the robot.", + ) + name: str = Field(..., description="Name of the file, including the extension.") + + class CSVParameter(RTPBase): """A CSV file parameter defined in a protocol.""" type: Literal["csv_file"] = Field( default="csv_file", description="String specifying the type of this parameter" ) - fileId: Optional[str] = Field( + file: Optional[FileInfo] = Field( default=None, description="ID of the CSV file stored on the robot; to be used for fetching the CSV file." " For local analysis this will most likely be empty.", diff --git a/api/src/opentrons/protocols/parameters/csv_parameter_definition.py b/api/src/opentrons/protocols/parameters/csv_parameter_definition.py index b6492b2ae54..342f4e1f180 100644 --- a/api/src/opentrons/protocols/parameters/csv_parameter_definition.py +++ b/api/src/opentrons/protocols/parameters/csv_parameter_definition.py @@ -4,6 +4,7 @@ from opentrons.protocol_engine.types import ( RunTimeParameter, CSVParameter as ProtocolEngineCSVParameter, + FileInfo, ) from . import validation @@ -28,7 +29,7 @@ def __init__( self._variable_name = validation.ensure_variable_name(variable_name) self._description = validation.ensure_description(description) self._value: Optional[TextIO] = None - self._id: Optional[str] = None + self._file_info: Optional[FileInfo] = None @property def variable_name(self) -> str: @@ -45,12 +46,12 @@ def value(self, new_file: TextIO) -> None: self._value = new_file @property - def id(self) -> Optional[str]: - return self._id + def file_info(self) -> Optional[FileInfo]: + return self._file_info - @id.setter - def id(self, uuid: str) -> None: - self._id = uuid + @file_info.setter + def file_info(self, file_info: FileInfo) -> None: + self._file_info = file_info def as_csv_parameter_interface(self) -> CSVParameter: return CSVParameter(csv_file=self._value) @@ -61,7 +62,7 @@ def as_protocol_engine_type(self) -> RunTimeParameter: displayName=self._display_name, variableName=self._variable_name, description=self._description, - fileId=self._id, + file=self._file_info, ) diff --git a/api/tests/opentrons/protocols/parameters/test_csv_parameter_definition.py b/api/tests/opentrons/protocols/parameters/test_csv_parameter_definition.py index 1c8c8638d53..0bb257cabfd 100644 --- a/api/tests/opentrons/protocols/parameters/test_csv_parameter_definition.py +++ b/api/tests/opentrons/protocols/parameters/test_csv_parameter_definition.py @@ -6,7 +6,7 @@ import pytest from decoy import Decoy -from opentrons.protocol_engine.types import CSVParameter +from opentrons.protocol_engine.types import CSVParameter, FileInfo from opentrons.protocols.parameters import validation as mock_validation from opentrons.protocols.parameters.csv_parameter_definition import ( create_csv_parameter, @@ -49,7 +49,7 @@ def test_create_csv_parameter(decoy: Decoy) -> None: assert result.variable_name == "my_cool_csv" assert result._description == "Comma Separated Value" assert result.value is None - assert result.id is None + assert result.file_info is None def test_set_csv_value( @@ -72,16 +72,16 @@ def test_csv_parameter_as_protocol_engine_type( displayName="My cool CSV", variableName="my_cool_csv", description="Comma Separated Value", - fileId=None, + file=None, ) - csv_parameter_subject.id = "123abc" + csv_parameter_subject.file_info = FileInfo(id="123abc", name="") result = csv_parameter_subject.as_protocol_engine_type() assert result == CSVParameter( displayName="My cool CSV", variableName="my_cool_csv", description="Comma Separated Value", - fileId="123abc", + file=FileInfo(id="123abc", name=""), ) diff --git a/robot-server/robot_server/protocols/analysis_store.py b/robot-server/robot_server/protocols/analysis_store.py index ae0f4aebbab..9af01d4227c 100644 --- a/robot-server/robot_server/protocols/analysis_store.py +++ b/robot-server/robot_server/protocols/analysis_store.py @@ -351,7 +351,7 @@ def _extract_csv_run_time_params( CsvParameterResource( analysis_id=completed_analysis.id, parameter_variable_name=param.variableName, - file_id=param.fileId, + file_id=param.file.id if param.file else None, ) for param in csv_rtp_list if isinstance(param, CSVParameter) @@ -402,7 +402,8 @@ async def matching_rtp_values_in_analysis( ), "Mismatch in parameters found in the current request vs. last saved parameters." # Indicates internal bug for param in new_parameters: if isinstance(param, CSVParameter): - if csv_rtps_in_last_analysis[param.variableName] != param.fileId: + new_file_id = param.file.id if param.file else None + if csv_rtps_in_last_analysis[param.variableName] != new_file_id: return False elif primitive_rtps_in_last_analysis[param.variableName] != param.value: return False diff --git a/robot-server/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml index 6acc8cac361..d7cbd6563c8 100644 --- a/robot-server/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml +++ b/robot-server/tests/integration/http_api/protocols/test_analyses_with_csv_file_parameters.tavern.yaml @@ -94,4 +94,6 @@ stages: - displayName: Liquid handling CSV file variableName: liq_handling_csv_file description: A CSV file that contains wells to use for pipetting - fileId: '{csv_file_id}' + file: + - id: '{csv_file_id}' + name: '' diff --git a/robot-server/tests/protocols/test_analysis_store.py b/robot-server/tests/protocols/test_analysis_store.py index 787ca752e92..8426d07ec4e 100644 --- a/robot-server/tests/protocols/test_analysis_store.py +++ b/robot-server/tests/protocols/test_analysis_store.py @@ -14,6 +14,7 @@ EnumChoice, BooleanParameter, CSVParameter, + FileInfo, ) from sqlalchemy.engine import Engine as SQLEngine @@ -112,12 +113,12 @@ def mock_number_param(name: str, value: float) -> NumberParameter: ) -def mock_csv_param(name: str, file_id: Optional[str]) -> CSVParameter: +def mock_csv_param(name: str, file: Optional[FileInfo]) -> CSVParameter: """Return a CSVParameter.""" return CSVParameter( variableName=name, displayName="csv param", - fileId=file_id, + file=file, ) @@ -342,7 +343,9 @@ async def test_update_adds_rtp_values_to_completed_store( default="blah", ) csv_param = pe_types.CSVParameter( - displayName="A CSV param", variableName="coolest_param", fileId="file-id" + displayName="A CSV param", + variableName="coolest_param", + file=FileInfo(id="file-id", name="file-name"), ) expected_completed_analysis_resource = CompletedAnalysisResource( id="analysis-id", @@ -545,7 +548,9 @@ async def test_save_initialization_failed_analysis( mock_number_param("cool_param", 2.0), mock_enum_param("cooler_param", "baz"), mock_bool_param("uncool_param", True), - mock_csv_param("coolest_param", "file-id"), + mock_csv_param( + "coolest_param", FileInfo(id="file-id", name="file-name") + ), ], True, ), @@ -554,7 +559,9 @@ async def test_save_initialization_failed_analysis( mock_number_param("cool_param", 2), mock_enum_param("cooler_param", "buzzzzz"), mock_bool_param("uncool_param", False), - mock_csv_param("coolest_param", "file-id"), + mock_csv_param( + "coolest_param", FileInfo(id="file-id", name="file-name") + ), ], False, ), @@ -563,7 +570,9 @@ async def test_save_initialization_failed_analysis( # params in different order, cool param is '2' instead of '2.0' mock_enum_param("cooler_param", "baz"), mock_bool_param("uncool_param", True), - mock_csv_param("coolest_param", "file-id"), + mock_csv_param( + "coolest_param", FileInfo(id="file-id", name="file-name") + ), mock_number_param("cool_param", 2), ], True, diff --git a/robot-server/tests/protocols/test_protocols_router.py b/robot-server/tests/protocols/test_protocols_router.py index a1a7de6085f..f951f5c8710 100644 --- a/robot-server/tests/protocols/test_protocols_router.py +++ b/robot-server/tests/protocols/test_protocols_router.py @@ -13,6 +13,7 @@ NumberParameter, CSVParameter, CsvRunTimeParamFilesType, + FileInfo, ) from opentrons.protocols.api_support.types import APIVersion @@ -1694,7 +1695,9 @@ async def test_update_protocol_analyses_with_new_rtp_values( default=3.0, ) csv_parameter = CSVParameter( - displayName="CSV parameter", variableName="csv_param", fileId="file-id" + displayName="CSV parameter", + variableName="csv_param", + file=FileInfo(id="file-id", name=""), ) decoy.when(protocol_store.has(protocol_id="protocol-id")).then_return(True) decoy.when(protocol_store.get(protocol_id="protocol-id")).then_return( diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index 758ee54663f..627859da19a 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -156,7 +156,6 @@ def run_time_parameters() -> List[pe_types.RunTimeParameter]: displayName="Display Name 4", variableName="variable_name_4", description="a csv parameter without file id", - fileId=None, ), ]