Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix invalid test default values and allow double/float parameters take integers #832

Merged
merged 4 commits into from
Aug 8, 2024
Merged

Fix invalid test default values and allow double/float parameters take integers #832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
815e584
Allow double/float parameters take integers and validate TYPE_MESSAGE…
LazeringDeath Aug 6, 2024
a8c30a6
get_type_default takes default values type, metadata handles default …
LazeringDeath Aug 7, 2024
c01b8b4
Implemented handling of empty lists
LazeringDeath Aug 7, 2024
5d24d44
Skip validation when default_value is an empty list
LazeringDeath Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any
from typing import Any, Optional

from google.protobuf.descriptor_pb2 import FieldDescriptorProto
from google.protobuf.type_pb2 import Field
Expand All @@ -15,6 +15,8 @@
Field.TYPE_ENUM: int(),
}

_PYTHON_DEFAULT_TYPES = set(type(value) for value in _TYPE_DEFAULT_MAPPING.values())

TYPE_FIELD_MAPPING = {
Field.TYPE_FLOAT: FieldDescriptorProto.TYPE_FLOAT,
Field.TYPE_DOUBLE: FieldDescriptorProto.TYPE_DOUBLE,
Expand All @@ -29,8 +31,16 @@
}


def get_type_default(type: Field.Kind.ValueType, repeated: bool) -> Any:
def get_type_default(
value_type: Field.Kind.ValueType, repeated: bool, default_value_type: Optional[type] = None
) -> Any:
"""Get the default value for the give type."""
if repeated:
return list()
return _TYPE_DEFAULT_MAPPING.get(type)
if (
value_type == Field.TYPE_MESSAGE
and default_value_type not in _PYTHON_DEFAULT_TYPES
and default_value_type is not None
):
return default_value_type()
return _TYPE_DEFAULT_MAPPING.get(value_type)
4 changes: 2 additions & 2 deletions packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/encoder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ def serialize_parameters(
parameter_metadata = parameter_metadata_dict[i]
field_name = parameter_metadata.field_name
parameter = _get_enum_values(param=parameter)
type_default_value = get_type_default(parameter_metadata.type, parameter_metadata.repeated)
default_value = get_type_default(parameter_metadata.type, parameter_metadata.repeated)

# Doesn't assign default values or None values to fields
if parameter != type_default_value and parameter is not None:
if parameter != default_value and parameter is not None:
if parameter_metadata.repeated:
getattr(message_instance, field_name).extend(parameter)
elif parameter_metadata.type == FieldDescriptorProto.TYPE_MESSAGE:
Expand Down
34 changes: 20 additions & 14 deletions packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import json
from enum import Enum
from typing import Any, Dict, Iterable, NamedTuple, Union, Type, Optional, TYPE_CHECKING
from typing import TYPE_CHECKING, Any, Dict, Iterable, NamedTuple, Optional, Type, Union

from google.protobuf import type_pb2

Expand All @@ -17,7 +17,6 @@
)
from ni_measurement_plugin_sdk_service.measurement.info import TypeSpecialization


if TYPE_CHECKING:
from google.protobuf.internal.enum_type_wrapper import _EnumTypeWrapper

Expand Down Expand Up @@ -60,7 +59,6 @@ class ParameterMetadata(NamedTuple):

@staticmethod
def initialize(
validate_type: bool,
display_name: str,
type: type_pb2.Field.Kind.ValueType,
repeated: bool,
Expand Down Expand Up @@ -89,8 +87,7 @@ def initialize(
field_name,
enum_type,
)
if validate_type:
_validate_default_value_type(parameter_metadata)
_validate_default_value_type(parameter_metadata)
return parameter_metadata


Expand Down Expand Up @@ -121,19 +118,26 @@ def _validate_default_value_type(parameter_metadata: ParameterMetadata) -> None:
if default_value is None:
return None

expected_type = type(get_type_default(parameter_metadata.type, parameter_metadata.repeated))
expected_type = type(
get_type_default(parameter_metadata.type, parameter_metadata.repeated, type(default_value))
)
display_name = parameter_metadata.display_name
enum_values_annotation = get_enum_values_annotation(parameter_metadata)

if parameter_metadata.repeated:
expected_element_type = type(get_type_default(parameter_metadata.type, False))
_validate_default_value_type_for_repeated_type(
default_value,
expected_type,
expected_element_type,
enum_values_annotation,
display_name,
)
if not isinstance(default_value, list):
raise TypeError(f"The default value '{default_value}' is not a list.")
if len(default_value) > 0:
expected_element_type = type(
get_type_default(parameter_metadata.type, False, type(default_value[0]))
)
_validate_default_value_type_for_repeated_type(
default_value,
expected_type,
expected_element_type,
enum_values_annotation,
display_name,
)
else:
_validate_default_value_type_for_scalar_type(
default_value, expected_type, enum_values_annotation, display_name
Expand Down Expand Up @@ -186,6 +190,8 @@ def _validate_default_value_type_for_basic_type(
display_name: str,
) -> None:
if not isinstance(default_value, expected_type):
if expected_type is float and isinstance(default_value, int):
return None
raise TypeError(
f"Unexpected type {type(default_value)} in the default value for '{display_name}'. Expected type: {expected_type}."
)
Expand Down
2 changes: 0 additions & 2 deletions packages/service/ni_measurement_plugin_sdk_service/measurement/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ def configuration(
data_type_info.type_specialization, instrument_type=instrument_type, enum_type=enum_type
)
parameter = parameter_metadata.ParameterMetadata.initialize(
True,
display_name,
data_type_info.grpc_field_type,
data_type_info.repeated,
Expand Down Expand Up @@ -471,7 +470,6 @@ def output(
data_type_info.type_specialization, enum_type=enum_type
)
parameter = parameter_metadata.ParameterMetadata.initialize(
False,
display_name,
data_type_info.grpc_field_type,
data_type_info.repeated,
Expand Down
23 changes: 0 additions & 23 deletions packages/service/tests/unit/test_decoder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,135 +176,118 @@ def _get_grpc_serialized_data(values):
def _get_test_parameter_by_id(default_values):
parameter_by_id = {
1: ParameterMetadata.initialize(
validate_type=False,
display_name="float_data",
type=type_pb2.Field.TYPE_FLOAT,
repeated=False,
default_value=default_values[0],
annotations={},
),
2: ParameterMetadata.initialize(
validate_type=False,
display_name="double_data",
type=type_pb2.Field.TYPE_DOUBLE,
repeated=False,
default_value=default_values[1],
annotations={},
),
3: ParameterMetadata.initialize(
validate_type=False,
display_name="int32_data",
type=type_pb2.Field.TYPE_INT32,
repeated=False,
default_value=default_values[2],
annotations={},
),
4: ParameterMetadata.initialize(
validate_type=False,
display_name="uint32_data",
type=type_pb2.Field.TYPE_INT64,
repeated=False,
default_value=default_values[3],
annotations={},
),
5: ParameterMetadata.initialize(
validate_type=False,
display_name="int64_data",
type=type_pb2.Field.TYPE_UINT32,
repeated=False,
default_value=default_values[4],
annotations={},
),
6: ParameterMetadata.initialize(
validate_type=False,
display_name="uint64_data",
type=type_pb2.Field.TYPE_UINT64,
repeated=False,
default_value=default_values[5],
annotations={},
),
7: ParameterMetadata.initialize(
validate_type=False,
display_name="bool_data",
type=type_pb2.Field.TYPE_BOOL,
repeated=False,
default_value=default_values[6],
annotations={},
),
8: ParameterMetadata.initialize(
validate_type=False,
display_name="string_data",
type=type_pb2.Field.TYPE_STRING,
repeated=False,
default_value=default_values[7],
annotations={},
),
9: ParameterMetadata.initialize(
validate_type=False,
display_name="double_array_data",
type=type_pb2.Field.TYPE_DOUBLE,
repeated=True,
default_value=default_values[8],
annotations={},
),
10: ParameterMetadata.initialize(
validate_type=False,
display_name="float_array_data",
type=type_pb2.Field.TYPE_FLOAT,
repeated=True,
default_value=default_values[9],
annotations={},
),
11: ParameterMetadata.initialize(
validate_type=False,
display_name="int32_array_data",
type=type_pb2.Field.TYPE_INT32,
repeated=True,
default_value=default_values[10],
annotations={},
),
12: ParameterMetadata.initialize(
validate_type=False,
display_name="uint32_array_data",
type=type_pb2.Field.TYPE_UINT32,
repeated=True,
default_value=default_values[11],
annotations={},
),
13: ParameterMetadata.initialize(
validate_type=False,
display_name="int64_array_data",
type=type_pb2.Field.TYPE_INT64,
repeated=True,
default_value=default_values[12],
annotations={},
),
14: ParameterMetadata.initialize(
validate_type=False,
display_name="uint64_array_data",
type=type_pb2.Field.TYPE_UINT64,
repeated=True,
default_value=default_values[13],
annotations={},
),
15: ParameterMetadata.initialize(
validate_type=False,
display_name="bool_array_data",
type=type_pb2.Field.TYPE_BOOL,
repeated=True,
default_value=default_values[14],
annotations={},
),
16: ParameterMetadata.initialize(
validate_type=False,
display_name="string_array_data",
type=type_pb2.Field.TYPE_STRING,
repeated=True,
default_value=default_values[15],
annotations={},
),
17: ParameterMetadata.initialize(
validate_type=False,
display_name="enum_data",
type=type_pb2.Field.TYPE_ENUM,
repeated=False,
Expand All @@ -316,7 +299,6 @@ def _get_test_parameter_by_id(default_values):
enum_type=DifferentColor,
),
18: ParameterMetadata.initialize(
validate_type=False,
display_name="enum_array_data",
type=type_pb2.Field.TYPE_ENUM,
repeated=True,
Expand All @@ -328,7 +310,6 @@ def _get_test_parameter_by_id(default_values):
enum_type=DifferentColor,
),
19: ParameterMetadata.initialize(
validate_type=False,
display_name="int_enum_data",
type=type_pb2.Field.TYPE_ENUM,
repeated=False,
Expand All @@ -340,7 +321,6 @@ def _get_test_parameter_by_id(default_values):
enum_type=Countries,
),
20: ParameterMetadata.initialize(
validate_type=False,
display_name="int_enum_array_data",
type=type_pb2.Field.TYPE_ENUM,
repeated=True,
Expand All @@ -352,7 +332,6 @@ def _get_test_parameter_by_id(default_values):
enum_type=Countries,
),
21: ParameterMetadata.initialize(
validate_type=False,
display_name="xy_data",
type=type_pb2.Field.TYPE_MESSAGE,
repeated=False,
Expand All @@ -361,7 +340,6 @@ def _get_test_parameter_by_id(default_values):
message_type=xydata_pb2.DoubleXYData.DESCRIPTOR.full_name,
),
22: ParameterMetadata.initialize(
validate_type=False,
display_name="xy_data_array",
type=type_pb2.Field.TYPE_MESSAGE,
repeated=True,
Expand Down Expand Up @@ -405,7 +383,6 @@ def _get_big_message_metadata_by_id() -> Dict[int, ParameterMetadata]:
return {
i
+ 1: ParameterMetadata.initialize(
validate_type=False,
display_name=f"field{i + 1}",
type=type_pb2.Field.TYPE_DOUBLE,
repeated=False,
Expand Down
11 changes: 10 additions & 1 deletion packages/service/tests/unit/test_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ class Countries(IntEnum):
(DataType.Boolean, 0, {}),
(DataType.String, True, {}),
(DataType.DoubleArray1D, 0.5, {}),
(DataType.Double, 1, {}),
LazeringDeath marked this conversation as resolved.
Show resolved Hide resolved
(
DataType.Enum,
1.0,
Expand Down Expand Up @@ -104,6 +103,8 @@ def test___default_value_different_from_type___validate___raises_type_exception(
(DataType.String, "string_default_value", {}),
(DataType.DoubleArray1D, [0.5, 0.1], {}),
LazeringDeath marked this conversation as resolved.
Show resolved Hide resolved
(DataType.Double, 1.0, {}),
(DataType.Double, 1, {}),
(DataType.DoubleArray1D, [], {}),
(
DataType.Enum,
Color.BLUE,
Expand Down Expand Up @@ -136,6 +137,14 @@ def test___default_value_different_from_type___validate___raises_type_exception(
ENUM_VALUES_KEY: '{"AMERICA":0, "TAIWAN": 1, "AUSTRALIA": 2, "CANADA": 3}',
},
),
(
DataType.EnumArray1D,
[],
{
TYPE_SPECIALIZATION_KEY: TypeSpecialization.Enum.value,
ENUM_VALUES_KEY: '{"NONE":0, "RED": 1, "GREEN": 2, "BLUE": 3}',
},
),
],
)
def test___default_value_same_as_type___validate___raises_no_exception(
Expand Down
2 changes: 1 addition & 1 deletion packages/service/tests/unit/test_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ def test___measurement_service___register_measurement_method___method_registered
("StringConfiguration", DataType.String, "DefaultString"),
("DoubleConfiguration", DataType.Double, 0.899),
("Float", DataType.Float, 0.100),
("IntFloat", DataType.Float, 1),
("Double1DArray", DataType.DoubleArray1D, [1.009, -1.0009]),
("Int32", DataType.Int32, -8799),
("Int64", DataType.Int64, -999),
Expand Down Expand Up @@ -281,7 +282,6 @@ def test___measurement_service___add_non_path_configuration__path_type_annotatio
("BoolConfiguration", DataType.Boolean, "MismatchDefaultValue"),
("StringConfiguration", DataType.String, True),
("DoubleConfiguration", DataType.Double, ""),
("Float", DataType.Float, 1),
LazeringDeath marked this conversation as resolved.
Show resolved Hide resolved
("Double1DArray", DataType.DoubleArray1D, ""),
("Int32", DataType.Int32, 1.0),
("Int64", DataType.Int64, 1.0),
Expand Down
Loading