Skip to content

Commit

Permalink
refactor(api): more clear error messages for type validation when cre…
Browse files Browse the repository at this point in the history
…ating runtime parameters (#14903)
  • Loading branch information
jbleon95 authored Apr 15, 2024
1 parent 4e895d4 commit 55f798a
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 27 deletions.
73 changes: 50 additions & 23 deletions api/src/opentrons/protocols/parameters/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,21 @@ def validate_variable_name_unique(

def ensure_display_name(display_name: str) -> str:
"""Validate display name is within the character limit."""
if not isinstance(display_name, str):
raise ParameterNameError(
f"Display name must be a string and at most {DISPLAY_NAME_MAX_LEN} characters."
)
if len(display_name) > DISPLAY_NAME_MAX_LEN:
raise ParameterNameError(
f"Display name {display_name} greater than {DISPLAY_NAME_MAX_LEN} characters."
f'Display name "{display_name}" greater than {DISPLAY_NAME_MAX_LEN} characters.'
)
return display_name


def ensure_variable_name(variable_name: str) -> str:
"""Validate variable name is a valid python variable name."""
if not isinstance(variable_name, str):
raise ParameterNameError("Variable name must be a string.")
if not variable_name.isidentifier():
raise ParameterNameError(
"Variable name must only contain alphanumeric characters, underscores, and cannot start with a digit."
Expand All @@ -49,19 +55,29 @@ def ensure_variable_name(variable_name: str) -> str:

def ensure_description(description: Optional[str]) -> Optional[str]:
"""Validate description is within the character limit."""
if description is not None and len(description) > DESCRIPTION_MAX_LEN:
raise ParameterNameError(
f"Description {description} greater than {DESCRIPTION_MAX_LEN} characters."
)
if description is not None:
if not isinstance(description, str):
raise ParameterNameError(
f"Description must be a string and at most {DESCRIPTION_MAX_LEN} characters."
)
if len(description) > DESCRIPTION_MAX_LEN:
raise ParameterNameError(
f'Description "{description}" greater than {DESCRIPTION_MAX_LEN} characters.'
)
return description


def ensure_unit_string_length(unit: Optional[str]) -> Optional[str]:
"""Validate unit is within the character limit."""
if unit is not None and len(unit) > UNIT_MAX_LEN:
raise ParameterNameError(
f"Description {unit} greater than {UNIT_MAX_LEN} characters."
)
if unit is not None:
if not isinstance(unit, str):
raise ParameterNameError(
f"Unit must be a string and at most {UNIT_MAX_LEN} characters."
)
if len(unit) > UNIT_MAX_LEN:
raise ParameterNameError(
f'Unit "{unit}" greater than {UNIT_MAX_LEN} characters.'
)
return unit


Expand Down Expand Up @@ -135,7 +151,7 @@ def convert_type_string_for_enum(
return "str"
else:
raise ParameterValueError(
f"Cannot resolve parameter type {parameter_type} for an enumerated parameter."
f"Cannot resolve parameter type '{parameter_type.__name__}' for an enumerated parameter."
)


Expand All @@ -147,7 +163,7 @@ def convert_type_string_for_num_param(parameter_type: type) -> Literal["int", "f
return "float"
else:
raise ParameterValueError(
f"Cannot resolve parameter type {parameter_type} for a number parameter."
f"Cannot resolve parameter type '{parameter_type.__name__}' for a number parameter."
)


Expand All @@ -173,7 +189,7 @@ def _validate_choices(
ensure_display_name(display_name)
if not isinstance(value, parameter_type):
raise ParameterDefinitionError(
f"All choices provided must match type {type(parameter_type)}"
f"All choices provided must be of type '{parameter_type.__name__}'"
)


Expand All @@ -192,29 +208,36 @@ def _validate_min_and_max(
"If a maximum value is provided a minimum must also be provided."
)
elif maximum is not None and minimum is not None:
if isinstance(maximum, (int, float)) and isinstance(minimum, (int, float)):
if maximum <= minimum:
if parameter_type is int or parameter_type is float:
if not isinstance(minimum, parameter_type):
raise ParameterDefinitionError(
"Maximum must be greater than the minimum"
f"Minimum is type '{type(minimum).__name__}',"
f" but must be of parameter type '{parameter_type.__name__}'"
)

if not isinstance(minimum, parameter_type) or not isinstance(
maximum, parameter_type
):
if not isinstance(maximum, parameter_type):
raise ParameterDefinitionError(
f"Minimum and maximum must match type {parameter_type}"
f"Maximum is type '{type(maximum).__name__}',"
f" but must be of parameter type '{parameter_type.__name__}'"
)
# These asserts are for the type checker and should never actually be asserted false
assert isinstance(minimum, (int, float))
assert isinstance(maximum, (int, float))
if maximum <= minimum:
raise ParameterDefinitionError(
"Maximum must be greater than the minimum"
)
else:
raise ParameterDefinitionError(
"Only parameters of type float or int can have a minimum and maximum"
"Only parameters of type float or int can have a minimum and maximum."
)


def validate_type(value: ParamType, parameter_type: type) -> None:
"""Validate parameter value is the correct type."""
if not isinstance(value, parameter_type):
raise ParameterValueError(
f"Parameter value {value} has type {type(value)}, must match type {parameter_type}."
f"Parameter value {value} has type '{type(value).__name__}',"
f" but must be of type '{parameter_type.__name__}'."
)


Expand All @@ -226,7 +249,11 @@ def validate_options(
parameter_type: type,
) -> None:
"""Validate default values and all possible constraints for a valid parameter definition."""
validate_type(default, parameter_type)
if not isinstance(default, parameter_type):
raise ParameterValueError(
f"Parameter default {default} has type '{type(default).__name__}',"
f" but must be of type '{parameter_type.__name__}'."
)

if choices is None and minimum is None and maximum is None:
raise ParameterDefinitionError(
Expand Down
9 changes: 5 additions & 4 deletions api/tests/opentrons/protocols/parameters/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,14 +278,15 @@ def test_convert_type_string_for_num_param_raises(param_type: type) -> None:
None,
[{"display_name": "abc", "value": "123"}],
int,
"must match type",
"must be of type",
),
(123, 1, None, None, int, "maximum must also"),
(123, None, 100, None, int, "minimum must also"),
(123, 100, 1, None, int, "Maximum must be greater"),
(123, 1.1, 100, None, int, "Minimum and maximum must match type"),
(123, 1, 100.5, None, int, "Minimum and maximum must match type"),
(123, "1", "100", None, int, "Only parameters of type float or int"),
(123, 1.1, 100, None, int, "Minimum is type"),
(123, 1, 100.5, None, int, "Maximum is type"),
(123.0, "1.0", 100.0, None, float, "Minimum is type"),
("blah", 1, 100, None, str, "Only parameters of type float or int"),
],
)
def test_validate_options_raise_definition_error(
Expand Down

0 comments on commit 55f798a

Please sign in to comment.