From 815e5847a79828250f7cb5989a32deb62a918780 Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Tue, 6 Aug 2024 11:46:02 -0500 Subject: [PATCH 1/4] Allow double/float parameters take integers and validate TYPE_MESSAGE type --- .../_internal/parameter/_get_type.py | 9 +++++++- .../_internal/parameter/encoder.py | 4 ++-- .../_internal/parameter/metadata.py | 17 ++++++++------ .../measurement/service.py | 2 -- packages/service/tests/unit/test_decoder.py | 23 ------------------- packages/service/tests/unit/test_metadata.py | 1 - packages/service/tests/unit/test_service.py | 1 - 7 files changed, 20 insertions(+), 37 deletions(-) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py index 916680889..888e569a2 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py @@ -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, @@ -29,8 +31,13 @@ } -def get_type_default(type: Field.Kind.ValueType, repeated: bool) -> Any: +def get_type_default(type: Field.Kind.ValueType, repeated: bool, default_value: Any = None) -> Any: """Get the default value for the give type.""" if repeated: return list() + if isinstance(default_value, list): + default_value = default_value[0] + + if type is Field.TYPE_MESSAGE and default_value.__class__ not in _PYTHON_DEFAULT_TYPES: + return default_value return _TYPE_DEFAULT_MAPPING.get(type) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/encoder.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/encoder.py index 0fd12ead9..c30e387f6 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/encoder.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/encoder.py @@ -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: diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py index 39ab664d5..92e924185 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py @@ -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 @@ -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 @@ -60,7 +59,6 @@ class ParameterMetadata(NamedTuple): @staticmethod def initialize( - validate_type: bool, display_name: str, type: type_pb2.Field.Kind.ValueType, repeated: bool, @@ -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 @@ -121,12 +118,16 @@ 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, 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)) + expected_element_type = type( + get_type_default(parameter_metadata.type, False, default_value) + ) _validate_default_value_type_for_repeated_type( default_value, expected_type, @@ -186,6 +187,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}." ) diff --git a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py index 7574be1b5..f223e1d8a 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py +++ b/packages/service/ni_measurement_plugin_sdk_service/measurement/service.py @@ -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, @@ -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, diff --git a/packages/service/tests/unit/test_decoder.py b/packages/service/tests/unit/test_decoder.py index 3c902329f..6add901d7 100644 --- a/packages/service/tests/unit/test_decoder.py +++ b/packages/service/tests/unit/test_decoder.py @@ -176,7 +176,6 @@ 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, @@ -184,7 +183,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 2: ParameterMetadata.initialize( - validate_type=False, display_name="double_data", type=type_pb2.Field.TYPE_DOUBLE, repeated=False, @@ -192,7 +190,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 3: ParameterMetadata.initialize( - validate_type=False, display_name="int32_data", type=type_pb2.Field.TYPE_INT32, repeated=False, @@ -200,7 +197,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 4: ParameterMetadata.initialize( - validate_type=False, display_name="uint32_data", type=type_pb2.Field.TYPE_INT64, repeated=False, @@ -208,7 +204,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 5: ParameterMetadata.initialize( - validate_type=False, display_name="int64_data", type=type_pb2.Field.TYPE_UINT32, repeated=False, @@ -216,7 +211,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 6: ParameterMetadata.initialize( - validate_type=False, display_name="uint64_data", type=type_pb2.Field.TYPE_UINT64, repeated=False, @@ -224,7 +218,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 7: ParameterMetadata.initialize( - validate_type=False, display_name="bool_data", type=type_pb2.Field.TYPE_BOOL, repeated=False, @@ -232,7 +225,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 8: ParameterMetadata.initialize( - validate_type=False, display_name="string_data", type=type_pb2.Field.TYPE_STRING, repeated=False, @@ -240,7 +232,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 9: ParameterMetadata.initialize( - validate_type=False, display_name="double_array_data", type=type_pb2.Field.TYPE_DOUBLE, repeated=True, @@ -248,7 +239,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 10: ParameterMetadata.initialize( - validate_type=False, display_name="float_array_data", type=type_pb2.Field.TYPE_FLOAT, repeated=True, @@ -256,7 +246,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 11: ParameterMetadata.initialize( - validate_type=False, display_name="int32_array_data", type=type_pb2.Field.TYPE_INT32, repeated=True, @@ -264,7 +253,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 12: ParameterMetadata.initialize( - validate_type=False, display_name="uint32_array_data", type=type_pb2.Field.TYPE_UINT32, repeated=True, @@ -272,7 +260,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 13: ParameterMetadata.initialize( - validate_type=False, display_name="int64_array_data", type=type_pb2.Field.TYPE_INT64, repeated=True, @@ -280,7 +267,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 14: ParameterMetadata.initialize( - validate_type=False, display_name="uint64_array_data", type=type_pb2.Field.TYPE_UINT64, repeated=True, @@ -288,7 +274,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 15: ParameterMetadata.initialize( - validate_type=False, display_name="bool_array_data", type=type_pb2.Field.TYPE_BOOL, repeated=True, @@ -296,7 +281,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 16: ParameterMetadata.initialize( - validate_type=False, display_name="string_array_data", type=type_pb2.Field.TYPE_STRING, repeated=True, @@ -304,7 +288,6 @@ def _get_test_parameter_by_id(default_values): annotations={}, ), 17: ParameterMetadata.initialize( - validate_type=False, display_name="enum_data", type=type_pb2.Field.TYPE_ENUM, repeated=False, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/packages/service/tests/unit/test_metadata.py b/packages/service/tests/unit/test_metadata.py index e5ca94dd0..3c9f6cca0 100644 --- a/packages/service/tests/unit/test_metadata.py +++ b/packages/service/tests/unit/test_metadata.py @@ -52,7 +52,6 @@ class Countries(IntEnum): (DataType.Boolean, 0, {}), (DataType.String, True, {}), (DataType.DoubleArray1D, 0.5, {}), - (DataType.Double, 1, {}), ( DataType.Enum, 1.0, diff --git a/packages/service/tests/unit/test_service.py b/packages/service/tests/unit/test_service.py index 46450ddd4..9e932f5ee 100644 --- a/packages/service/tests/unit/test_service.py +++ b/packages/service/tests/unit/test_service.py @@ -281,7 +281,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), ("Double1DArray", DataType.DoubleArray1D, ""), ("Int32", DataType.Int32, 1.0), ("Int64", DataType.Int64, 1.0), From a8c30a6832368a9b7e29a3187bf0f573ffa86b4c Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Wed, 7 Aug 2024 14:46:59 -0500 Subject: [PATCH 2/4] get_type_default takes default values type, metadata handles default lists better, fixed formatting --- .../_internal/parameter/_get_type.py | 19 +++++++++++-------- .../_internal/parameter/metadata.py | 11 +++++++---- packages/service/tests/unit/test_metadata.py | 1 + packages/service/tests/unit/test_service.py | 1 + 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py index 888e569a2..09c7ee05d 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/_get_type.py @@ -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 @@ -31,13 +31,16 @@ } -def get_type_default(type: Field.Kind.ValueType, repeated: bool, default_value: Any = None) -> 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() - if isinstance(default_value, list): - default_value = default_value[0] - - if type is Field.TYPE_MESSAGE and default_value.__class__ not in _PYTHON_DEFAULT_TYPES: - return default_value - 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) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py index 92e924185..e40b7ea5b 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py @@ -119,15 +119,18 @@ def _validate_default_value_type(parameter_metadata: ParameterMetadata) -> None: return None expected_type = type( - get_type_default(parameter_metadata.type, parameter_metadata.repeated, default_value) + 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, default_value) - ) + 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, diff --git a/packages/service/tests/unit/test_metadata.py b/packages/service/tests/unit/test_metadata.py index 3c9f6cca0..18117f159 100644 --- a/packages/service/tests/unit/test_metadata.py +++ b/packages/service/tests/unit/test_metadata.py @@ -103,6 +103,7 @@ def test___default_value_different_from_type___validate___raises_type_exception( (DataType.String, "string_default_value", {}), (DataType.DoubleArray1D, [0.5, 0.1], {}), (DataType.Double, 1.0, {}), + (DataType.Double, 1, {}), ( DataType.Enum, Color.BLUE, diff --git a/packages/service/tests/unit/test_service.py b/packages/service/tests/unit/test_service.py index 9e932f5ee..aa7ea2de0 100644 --- a/packages/service/tests/unit/test_service.py +++ b/packages/service/tests/unit/test_service.py @@ -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), From c01b8b420dfcc8e7670859583db59fd0be351acf Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Wed, 7 Aug 2024 17:51:01 -0500 Subject: [PATCH 3/4] Implemented handling of empty lists --- .../_internal/parameter/metadata.py | 2 ++ packages/service/tests/unit/test_metadata.py | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py index e40b7ea5b..357a66cec 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py @@ -131,6 +131,8 @@ def _validate_default_value_type(parameter_metadata: ParameterMetadata) -> None: expected_element_type = type( get_type_default(parameter_metadata.type, False, type(default_value[0])) ) + else: + expected_element_type = expected_type _validate_default_value_type_for_repeated_type( default_value, expected_type, diff --git a/packages/service/tests/unit/test_metadata.py b/packages/service/tests/unit/test_metadata.py index 18117f159..bb721bb2c 100644 --- a/packages/service/tests/unit/test_metadata.py +++ b/packages/service/tests/unit/test_metadata.py @@ -104,6 +104,7 @@ def test___default_value_different_from_type___validate___raises_type_exception( (DataType.DoubleArray1D, [0.5, 0.1], {}), (DataType.Double, 1.0, {}), (DataType.Double, 1, {}), + (DataType.DoubleArray1D, [], {}), ( DataType.Enum, Color.BLUE, @@ -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( From 5d24d44795d71e2eaee20fa2bfc76b4561c677e0 Mon Sep 17 00:00:00 2001 From: Tyler Nguyen Date: Thu, 8 Aug 2024 08:43:15 -0500 Subject: [PATCH 4/4] Skip validation when default_value is an empty list --- .../_internal/parameter/metadata.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py index 357a66cec..2e4fcb7ea 100644 --- a/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py +++ b/packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py @@ -131,15 +131,13 @@ def _validate_default_value_type(parameter_metadata: ParameterMetadata) -> None: expected_element_type = type( get_type_default(parameter_metadata.type, False, type(default_value[0])) ) - else: - expected_element_type = expected_type - _validate_default_value_type_for_repeated_type( - default_value, - expected_type, - expected_element_type, - enum_values_annotation, - display_name, - ) + _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