From 7ce24a583557379b50eb78ea58d6f350384d23b6 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Tue, 19 Mar 2024 10:13:42 -0400 Subject: [PATCH] changes per code review --- api/src/opentrons/protocol_api/_parameters.py | 7 +++---- .../opentrons/protocol_api/parameter_context.py | 8 ++++---- .../protocols/parameters/parameter_definition.py | 14 +++++++------- api/src/opentrons/protocols/parameters/types.py | 2 +- .../opentrons/protocols/parameters/validation.py | 6 +++--- .../protocols/parameters/test_validation.py | 4 ++-- 6 files changed, 20 insertions(+), 21 deletions(-) diff --git a/api/src/opentrons/protocol_api/_parameters.py b/api/src/opentrons/protocol_api/_parameters.py index 4470cb956d2..325cce42a7f 100644 --- a/api/src/opentrons/protocol_api/_parameters.py +++ b/api/src/opentrons/protocol_api/_parameters.py @@ -11,16 +11,15 @@ def __init__(self, parameters: Optional[Dict[str, AllowedTypes]] = None) -> None self._initialize_parameter(name, value) def _getparam(self, variable_name: str) -> Any: - return getattr(self, f"_{variable_name}") + return self._values[variable_name] def _initialize_parameter(self, variable_name: str, value: AllowedTypes) -> None: - if not hasattr(self, variable_name) and not hasattr(self, f"_{variable_name}"): - setattr(self, f"_{variable_name}", value) + if not hasattr(self, variable_name): + self._values[variable_name] = value prop = property( fget=lambda s, v=variable_name: Parameters._getparam(s, v) # type: ignore[misc] ) setattr(Parameters, variable_name, prop) - self._values[variable_name] = value else: raise ParameterNameError( f"Cannot use {variable_name} as a variable name, either duplicates another" diff --git a/api/src/opentrons/protocol_api/parameter_context.py b/api/src/opentrons/protocol_api/parameter_context.py index e3629c36e8a..6a503f7337a 100644 --- a/api/src/opentrons/protocol_api/parameter_context.py +++ b/api/src/opentrons/protocol_api/parameter_context.py @@ -4,7 +4,7 @@ from opentrons.protocols.api_support.types import APIVersion from opentrons.protocols.parameters import parameter_definition -from opentrons.protocols.parameters.types import ParameterChoices +from opentrons.protocols.parameters.types import ParameterChoice from ._parameters import Parameters @@ -31,7 +31,7 @@ def add_int( default: int, minimum: Optional[int] = None, maximum: Optional[int] = None, - choices: Optional[List[ParameterChoices]] = None, + choices: Optional[List[ParameterChoice]] = None, description: Optional[str] = None, unit: Optional[str] = None, ) -> None: @@ -68,7 +68,7 @@ def add_float( default: float, minimum: Optional[float] = None, maximum: Optional[float] = None, - choices: Optional[List[ParameterChoices]] = None, + choices: Optional[List[ParameterChoice]] = None, description: Optional[str] = None, unit: Optional[str] = None, ) -> None: @@ -131,7 +131,7 @@ def add_str( display_name: str, variable_name: str, default: str, - choices: Optional[List[ParameterChoices]] = None, + choices: Optional[List[ParameterChoice]] = None, description: Optional[str] = None, ) -> None: """Creates a string parameter, settable among given choices. diff --git a/api/src/opentrons/protocols/parameters/parameter_definition.py b/api/src/opentrons/protocols/parameters/parameter_definition.py index 124da341fcc..54c27b1840d 100644 --- a/api/src/opentrons/protocols/parameters/parameter_definition.py +++ b/api/src/opentrons/protocols/parameters/parameter_definition.py @@ -4,7 +4,7 @@ from opentrons.protocols.parameters.types import ( ParamType, - ParameterChoices, + ParameterChoice, AllowedTypes, ParameterDefinitionError, ParameterValueError, @@ -23,7 +23,7 @@ def __init__( default: ParamType, minimum: Optional[ParamType] = None, maximum: Optional[ParamType] = None, - choices: Optional[List[ParameterChoices]] = None, + choices: Optional[List[ParameterChoice]] = None, description: Optional[str] = None, unit: Optional[str] = None, ) -> None: @@ -57,7 +57,7 @@ def __init__( ) self._type = parameter_type - self._choices: Optional[List[ParameterChoices]] = choices + self._choices: Optional[List[ParameterChoice]] = choices self._allowed_values: Optional[Set[AllowedTypes]] = None self._minimum: Optional[Union[int, float]] = None @@ -111,7 +111,7 @@ def create_int_parameter( default: int, minimum: Optional[int] = None, maximum: Optional[int] = None, - choices: Optional[List[ParameterChoices]] = None, + choices: Optional[List[ParameterChoice]] = None, description: Optional[str] = None, unit: Optional[str] = None, ) -> ParameterDefinition[int]: @@ -135,7 +135,7 @@ def create_float_parameter( default: float, minimum: Optional[float] = None, maximum: Optional[float] = None, - choices: Optional[List[ParameterChoices]] = None, + choices: Optional[List[ParameterChoice]] = None, description: Optional[str] = None, unit: Optional[str] = None, ) -> ParameterDefinition[float]: @@ -157,7 +157,7 @@ def create_bool_parameter( display_name: str, variable_name: str, default: bool, - choices: List[ParameterChoices], + choices: List[ParameterChoice], description: Optional[str] = None, ) -> ParameterDefinition[bool]: """Creates a boolean parameter.""" @@ -175,7 +175,7 @@ def create_str_parameter( display_name: str, variable_name: str, default: str, - choices: Optional[List[ParameterChoices]] = None, + choices: Optional[List[ParameterChoice]] = None, description: Optional[str] = None, ) -> ParameterDefinition[str]: """Creates a string parameter.""" diff --git a/api/src/opentrons/protocols/parameters/types.py b/api/src/opentrons/protocols/parameters/types.py index d79e6743085..7edf0c941d5 100644 --- a/api/src/opentrons/protocols/parameters/types.py +++ b/api/src/opentrons/protocols/parameters/types.py @@ -6,7 +6,7 @@ ParamType = TypeVar("ParamType", bound=AllowedTypes) -class ParameterChoices(TypedDict): +class ParameterChoice(TypedDict): """A parameter choice containing the display name and value.""" display_name: str diff --git a/api/src/opentrons/protocols/parameters/validation.py b/api/src/opentrons/protocols/parameters/validation.py index 1ca08aa8ba4..9b4cae7354e 100644 --- a/api/src/opentrons/protocols/parameters/validation.py +++ b/api/src/opentrons/protocols/parameters/validation.py @@ -3,7 +3,7 @@ from .types import ( ParamType, - ParameterChoices, + ParameterChoice, ParameterNameError, ParameterValueError, ParameterDefinitionError, @@ -56,7 +56,7 @@ def ensure_unit_string_length(unit: Optional[str]) -> Optional[str]: def _validate_choices( minimum: Optional[ParamType], maximum: Optional[ParamType], - choices: List[ParameterChoices], + choices: List[ParameterChoice], parameter_type: type, ) -> None: """Validate that min and max is not defined and all choices are properly formatted.""" @@ -124,7 +124,7 @@ def validate_options( default: ParamType, minimum: Optional[ParamType], maximum: Optional[ParamType], - choices: Optional[List[ParameterChoices]], + choices: Optional[List[ParameterChoice]], parameter_type: type, ) -> None: """Validate default values and all possible constraints for a valid parameter definition.""" diff --git a/api/tests/opentrons/protocols/parameters/test_validation.py b/api/tests/opentrons/protocols/parameters/test_validation.py index 29b2b58f1ae..cd82fe173c4 100644 --- a/api/tests/opentrons/protocols/parameters/test_validation.py +++ b/api/tests/opentrons/protocols/parameters/test_validation.py @@ -3,7 +3,7 @@ from opentrons.protocols.parameters.types import ( AllowedTypes, - ParameterChoices, + ParameterChoice, ParameterNameError, ParameterValueError, ParameterDefinitionError, @@ -171,7 +171,7 @@ def test_validate_options_raise_definition_error( default: AllowedTypes, minimum: Optional[AllowedTypes], maximum: Optional[AllowedTypes], - choices: Optional[List[ParameterChoices]], + choices: Optional[List[ParameterChoice]], parameter_type: type, error_text: str, ) -> None: