From 6643e6b375cd87675617b6878c0a5b5843a3bec4 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Tue, 13 May 2025 16:52:24 -0500 Subject: [PATCH 01/39] First draft of builtin type marshalling Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- CONTRIBUTING.md | 3 + examples/placeholder.py | 59 ++++++++++++++++ src/nipanel/_converters.py | 107 +++++++++++++++++++++++++++++ src/nipanel/_panel.py | 6 +- src/nipanel/_panel_client.py | 38 +++++++++- tests/unit/test_streamlit_panel.py | 14 ++-- 6 files changed, 216 insertions(+), 11 deletions(-) create mode 100644 src/nipanel/_converters.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 25621eb..219ed74 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,6 +43,9 @@ poetry run nps lint poetry run mypy poetry run bandit -c pyproject.toml -r src/nipanel +# Apply safe fixes +poetry run nps fix + # Run the tests poetry run pytest -v diff --git a/examples/placeholder.py b/examples/placeholder.py index 40eab88..147e118 100644 --- a/examples/placeholder.py +++ b/examples/placeholder.py @@ -1 +1,60 @@ """Placeholder example for the package.""" + +import enum + +import nipanel + + +class MyIntFlags(enum.IntFlag): + """Example of an IntFlag enum.""" + + VALUE1 = 1 + VALUE2 = 2 + VALUE4 = 4 + + +class MyIntEnum(enum.IntEnum): + """Example of an IntEnum enum.""" + + VALUE10 = 10 + VALUE20 = 20 + VALUE30 = 30 + + +class MyStrEnum(enum.StrEnum): + """Example of a StrEnum enum.""" + + VALUE1 = "value1" + VALUE2 = "value2" + VALUE3 = "value3" + + +if __name__ == "__main__": + my_panel = nipanel.Panel( + panel_id="placeholder", + panel_uri=__file__, + provided_interface="ni.pythonpanel.v1.PythonPanelService", + service_class="ni.pythonpanel.v1.PythonPanelService", + ) + + my_types = { + "my_str": "im justa smol str", + "my_int": 42, + "my_float": 13.12, + "my_bool": True, + "my_bytes": b"robotext", + "my_intflags": MyIntFlags.VALUE1 | MyIntFlags.VALUE4, + "my_intenum": MyIntEnum.VALUE20, + "my_strenum": MyStrEnum.VALUE3, + } + + print("Setting values") + for name, value in my_types.items(): + print(f"{name:>15} {value}") + my_panel.set_value(name, value) + + print() + print("Getting values") + for name in my_types.keys(): + the_value = my_panel.get_value(name) + print(f"{name:>15} {the_value}") diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py new file mode 100644 index 0000000..ac35d8a --- /dev/null +++ b/src/nipanel/_converters.py @@ -0,0 +1,107 @@ +"""Functions to convert between different data formats.""" + +from __future__ import annotations + +import logging +import typing + +import google.protobuf.any_pb2 +import google.protobuf.message +import google.protobuf.wrappers_pb2 + +_logger = logging.getLogger(__name__) + + +class ConvertibleType(typing.NamedTuple): + """A Python type that can be converted to and from a protobuf Any.""" + + name: str + """The human name of the type.""" + + python_typename: str + """The Python name for the type.""" + + protobuf_typename: str + """The protobuf name for the type.""" + + protobuf_initializer: typing.Callable[..., google.protobuf.message.Message] + """A callable that can be used to create an instance of the protobuf type.""" + + +_CONVERTIBLE_TYPES: dict[str, ConvertibleType] = { + "bool": ConvertibleType( + name="Boolean", + python_typename=bool.__name__, + protobuf_typename=google.protobuf.wrappers_pb2.BoolValue.DESCRIPTOR.full_name, + protobuf_initializer=google.protobuf.wrappers_pb2.BoolValue, + ), + "bytes": ConvertibleType( + name="Bytes", + python_typename=bytes.__name__, + protobuf_typename=google.protobuf.wrappers_pb2.BytesValue.DESCRIPTOR.full_name, + protobuf_initializer=google.protobuf.wrappers_pb2.BytesValue, + ), + "float": ConvertibleType( + name="Float", + python_typename=float.__name__, + protobuf_typename=google.protobuf.wrappers_pb2.DoubleValue.DESCRIPTOR.full_name, + protobuf_initializer=google.protobuf.wrappers_pb2.DoubleValue, + ), + "int": ConvertibleType( + name="Integer", + python_typename=int.__name__, + protobuf_typename=google.protobuf.wrappers_pb2.Int64Value.DESCRIPTOR.full_name, + protobuf_initializer=google.protobuf.wrappers_pb2.Int64Value, + ), + "str": ConvertibleType( + name="String", + python_typename=str.__name__, + protobuf_typename=google.protobuf.wrappers_pb2.StringValue.DESCRIPTOR.full_name, + protobuf_initializer=google.protobuf.wrappers_pb2.StringValue, + ), +} + + +def to_any(python_value: object) -> google.protobuf.any_pb2.Any: + """Convert a Python object to a protobuf Any.""" + protobuf_wrapper_for_type = { + entry.python_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES.values() + } + supported_types = set(protobuf_wrapper_for_type.keys()) + underlying_parents = [ + parent.__name__ + for parent in type(python_value).mro() # This covers enum.IntEnum and similar + ] + + best_matching_type = next( + (parent for parent in underlying_parents if parent in supported_types), None + ) + if not best_matching_type: + raise TypeError( + f"Unsupported type: {type(python_value)} with parents {underlying_parents}. Supported types are: {supported_types}" + ) + _logger.debug(f"Best matching type for '{repr(python_value)}' resolved to {best_matching_type}") + + the_any = google.protobuf.any_pb2.Any() + wrapped_value = protobuf_wrapper_for_type[best_matching_type](value=python_value) + the_any.Pack(wrapped_value) + return the_any + + +def from_any(protobuf_any: google.protobuf.any_pb2.Any) -> object: + """Convert a protobuf Any to a Python object.""" + protobuf_wrapper_for_type = { + entry.protobuf_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES.values() + } + if not isinstance(protobuf_any, google.protobuf.any_pb2.Any): + raise ValueError(f"Unexpected type: {type(protobuf_any)}") + + underlying_typename = protobuf_any.TypeName() + _logger.debug(f"Unpacking type '{underlying_typename}'") + + protobuf_wrapper = protobuf_wrapper_for_type[underlying_typename]() + did_unpack = protobuf_any.Unpack(protobuf_wrapper) + if not did_unpack: + raise ValueError(f"Failed to unpack Any with underlying type '{underlying_typename}'") + + return protobuf_wrapper.value diff --git a/src/nipanel/_panel.py b/src/nipanel/_panel.py index 4e6ca3a..41d0d0b 100644 --- a/src/nipanel/_panel.py +++ b/src/nipanel/_panel.py @@ -63,8 +63,7 @@ def get_value(self, value_id: str) -> object: Returns: The value """ - # TODO: AB#3095681 - get the Any from _client.get_value and convert it to the correct type - return "placeholder value" + return self._panel_client.get_value(self._panel_id, value_id) def set_value(self, value_id: str, value: object) -> None: """Set the value for a control on the panel. @@ -73,5 +72,4 @@ def set_value(self, value_id: str, value: object) -> None: value_id: The id of the value value: The value """ - # TODO: AB#3095681 - Convert the value to an Any and pass it to _client.set_value - pass + self._panel_client.set_value(self._panel_id, value_id, value) diff --git a/src/nipanel/_panel_client.py b/src/nipanel/_panel_client.py index 0d37bfa..82b21c4 100644 --- a/src/nipanel/_panel_client.py +++ b/src/nipanel/_panel_client.py @@ -7,12 +7,21 @@ from typing import Callable, TypeVar import grpc -from ni.pythonpanel.v1.python_panel_service_pb2 import OpenPanelRequest +from ni.pythonpanel.v1.python_panel_service_pb2 import ( + GetValueRequest, + OpenPanelRequest, + SetValueRequest, +) from ni.pythonpanel.v1.python_panel_service_pb2_grpc import PythonPanelServiceStub from ni_measurement_plugin_sdk_service.discovery import DiscoveryClient from ni_measurement_plugin_sdk_service.grpc.channelpool import GrpcChannelPool from typing_extensions import ParamSpec +from nipanel._converters import ( + from_any, + to_any, +) + _P = ParamSpec("_P") _T = TypeVar("_T") @@ -53,6 +62,33 @@ def open_panel(self, panel_id: str, panel_uri: str) -> None: open_panel_request = OpenPanelRequest(panel_id=panel_id, panel_uri=panel_uri) self._invoke_with_retry(self._get_stub().OpenPanel, open_panel_request) + def set_value(self, panel_id: str, value_id: str, value: object) -> None: + """Set the value for the control with value_id. + + Args: + panel_id: The ID of the panel. + value_id: The ID of the control. + value: The value to set. + """ + new_any = to_any(value) + set_value_request = SetValueRequest(panel_id=panel_id, value_id=value_id, value=new_any) + self._invoke_with_retry(self._get_stub().SetValue, set_value_request) + + def get_value(self, panel_id: str, value_id: str) -> object: + """Get the value for the control with value_id. + + Args: + panel_id: The ID of the panel. + value_id: The ID of the control. + + Returns: + The value. + """ + get_value_request = GetValueRequest(panel_id=panel_id, value_id=value_id) + response = self._invoke_with_retry(self._get_stub().GetValue, get_value_request) + the_value = from_any(response.value) + return the_value + def _get_stub(self) -> PythonPanelServiceStub: if self._stub is None: if self._grpc_channel is not None: diff --git a/tests/unit/test_streamlit_panel.py b/tests/unit/test_streamlit_panel.py index 3ffb5c2..e3c80f7 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -17,10 +17,11 @@ def test___opened_panel___set_value___gets_same_value( panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) panel.open_panel() - panel.set_value("test_id", "test_value") + value_id = "test_id" + string_value = "test_value" + panel.set_value(value_id, string_value) - # TODO: AB#3095681 - change asserted value to test_value - assert panel.get_value("test_id") == "placeholder value" + assert panel.get_value(value_id) == string_value def test___first_open_panel_fails___open_panel___gets_value( @@ -36,6 +37,7 @@ def test___first_open_panel_fails___open_panel___gets_value( panel.open_panel() - panel.set_value("test_id", "test_value") - # TODO: AB#3095681 - change asserted value to test_value - assert panel.get_value("test_id") == "placeholder value" + value_id = "test_id" + string_value = "test_value" + panel.set_value(value_id, string_value) + assert panel.get_value(value_id) == string_value From 8d97456d7fa424a06efa5ee9db41d358ef2c9dcf Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Wed, 14 May 2025 10:15:47 -0500 Subject: [PATCH 02/39] Use a brute-force explicit helper for type-hinting the convertible named tuple Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index ac35d8a..1ec09bf 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -12,6 +12,15 @@ _logger = logging.getLogger(__name__) +_builtin_protobuf_type = ( + google.protobuf.wrappers_pb2.BoolValue + | google.protobuf.wrappers_pb2.BytesValue + | google.protobuf.wrappers_pb2.DoubleValue + | google.protobuf.wrappers_pb2.Int64Value + | google.protobuf.wrappers_pb2.StringValue +) + + class ConvertibleType(typing.NamedTuple): """A Python type that can be converted to and from a protobuf Any.""" @@ -24,7 +33,7 @@ class ConvertibleType(typing.NamedTuple): protobuf_typename: str """The protobuf name for the type.""" - protobuf_initializer: typing.Callable[..., google.protobuf.message.Message] + protobuf_initializer: typing.Callable[..., _builtin_protobuf_type] """A callable that can be used to create an instance of the protobuf type.""" From 02c1cddb7d444314126c4b3161217d63195adbf4 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Wed, 14 May 2025 10:16:05 -0500 Subject: [PATCH 03/39] Use the StreamlitPanel as the main entrypoint Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- examples/placeholder.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/examples/placeholder.py b/examples/placeholder.py index 147e118..e9b1216 100644 --- a/examples/placeholder.py +++ b/examples/placeholder.py @@ -30,11 +30,9 @@ class MyStrEnum(enum.StrEnum): if __name__ == "__main__": - my_panel = nipanel.Panel( + my_panel = nipanel.StreamlitPanel( panel_id="placeholder", - panel_uri=__file__, - provided_interface="ni.pythonpanel.v1.PythonPanelService", - service_class="ni.pythonpanel.v1.PythonPanelService", + streamlit_script_uri=__file__, ) my_types = { From ec9e760db34c4c9c6a3906abbc3c9d9eab09d7e6 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Wed, 14 May 2025 10:16:26 -0500 Subject: [PATCH 04/39] Add tests for unopened-set, unopened-get, and scalar round trips Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/unit/test_streamlit_panel.py | 58 ++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/unit/test_streamlit_panel.py b/tests/unit/test_streamlit_panel.py index e3c80f7..0eb624b 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -41,3 +41,61 @@ def test___first_open_panel_fails___open_panel___gets_value( string_value = "test_value" panel.set_value(value_id, string_value) assert panel.get_value(value_id) == string_value + + +def test___unopened_panel___set_value___sets_value( + grpc_channel_for_fake_panel_service: grpc.Channel, +) -> None: + """Test that set_value() succeeds before the user opens the panel.""" + channel = grpc_channel_for_fake_panel_service + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + + value_id = "test_id" + string_value = "test_value" + panel.set_value(value_id, string_value) + + assert panel.get_value(value_id) == string_value + + +def test___unopened_panel___get_value___gets_value( + grpc_channel_for_fake_panel_service: grpc.Channel, +) -> None: + """Test that get_value() succeeds before the user opens the panel.""" + channel = grpc_channel_for_fake_panel_service + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + + value_id = "test_id" + string_value = "test_value" + panel.set_value(value_id, string_value) + + assert panel.get_value(value_id) == string_value + + +def test___builtin_scalar_types___set_value___gets_same_value( + grpc_channel_for_fake_panel_service: grpc.Channel, +) -> None: + """Test that set_value() and get_value() work for builtin scalar types.""" + channel = grpc_channel_for_fake_panel_service + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + + value_id = "test_id" + string_value = "test_value" + int_value = 42 + float_value = 3.14 + bool_value = True + bytes_value = b"robotext" + + panel.set_value(value_id, string_value) + assert panel.get_value(value_id) == string_value + + panel.set_value(value_id, int_value) + assert panel.get_value(value_id) == int_value + + panel.set_value(value_id, float_value) + assert panel.get_value(value_id) == float_value + + panel.set_value(value_id, bool_value) + assert panel.get_value(value_id) == bool_value + + panel.set_value(value_id, bytes_value) + assert panel.get_value(value_id) == bytes_value From 44076681b3ee98cc413d7dc037627383879e750a Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Wed, 14 May 2025 10:26:59 -0500 Subject: [PATCH 05/39] Remove unused import Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 1ec09bf..0050b66 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -6,7 +6,6 @@ import typing import google.protobuf.any_pb2 -import google.protobuf.message import google.protobuf.wrappers_pb2 _logger = logging.getLogger(__name__) From 86c22aadf470002228ca5bf3bfc7db16076a9139 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Wed, 14 May 2025 10:57:38 -0500 Subject: [PATCH 06/39] Add test showing an unset value raises when clients attempt to get it Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/unit/test_streamlit_panel.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_streamlit_panel.py b/tests/unit/test_streamlit_panel.py index 0eb624b..de121fd 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -1,4 +1,5 @@ import grpc +import pytest from nipanel._streamlit_panel import StreamlitPanel from tests.utils._fake_python_panel_service import FakePythonPanelService @@ -57,10 +58,22 @@ def test___unopened_panel___set_value___sets_value( assert panel.get_value(value_id) == string_value -def test___unopened_panel___get_value___gets_value( +def test___unopened_panel___get_unset_value___raises_exception( grpc_channel_for_fake_panel_service: grpc.Channel, ) -> None: - """Test that get_value() succeeds before the user opens the panel.""" + """Test that get_value() raises an exception for an unset value.""" + channel = grpc_channel_for_fake_panel_service + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + + value_id = "test_id" + with pytest.raises(grpc.RpcError): + panel.get_value(value_id) + + +def test___unopened_panel___get_set_value___gets_value( + grpc_channel_for_fake_panel_service: grpc.Channel, +) -> None: + """Test that get_value() succeeds for a set value before the user opens the panel.""" channel = grpc_channel_for_fake_panel_service panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) From b63ea7322783af0a3a17ea94d1c2b45dadde4c36 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 11:53:56 -0500 Subject: [PATCH 07/39] Add type hints that are compatible with Python 3.9 Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 0050b66..2c54921 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -3,24 +3,25 @@ from __future__ import annotations import logging -import typing +from typing import Callable, NamedTuple, Union import google.protobuf.any_pb2 import google.protobuf.wrappers_pb2 +from typing_extensions import TypeAlias _logger = logging.getLogger(__name__) -_builtin_protobuf_type = ( - google.protobuf.wrappers_pb2.BoolValue - | google.protobuf.wrappers_pb2.BytesValue - | google.protobuf.wrappers_pb2.DoubleValue - | google.protobuf.wrappers_pb2.Int64Value - | google.protobuf.wrappers_pb2.StringValue -) +_builtin_protobuf_type: TypeAlias = Union[ + google.protobuf.wrappers_pb2.BoolValue, + google.protobuf.wrappers_pb2.BytesValue, + google.protobuf.wrappers_pb2.DoubleValue, + google.protobuf.wrappers_pb2.Int64Value, + google.protobuf.wrappers_pb2.StringValue, +] -class ConvertibleType(typing.NamedTuple): +class ConvertibleType(NamedTuple): """A Python type that can be converted to and from a protobuf Any.""" name: str @@ -32,7 +33,7 @@ class ConvertibleType(typing.NamedTuple): protobuf_typename: str """The protobuf name for the type.""" - protobuf_initializer: typing.Callable[..., _builtin_protobuf_type] + protobuf_initializer: Callable[..., _builtin_protobuf_type] """A callable that can be used to create an instance of the protobuf type.""" From a60c17f78e3938dbb11c5f9be53ae9e1c05f7a9f Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 12:01:14 -0500 Subject: [PATCH 08/39] Do not add type hints when the analyzer can infer it Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 2c54921..773e7b9 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -37,7 +37,7 @@ class ConvertibleType(NamedTuple): """A callable that can be used to create an instance of the protobuf type.""" -_CONVERTIBLE_TYPES: dict[str, ConvertibleType] = { +_CONVERTIBLE_TYPES = { "bool": ConvertibleType( name="Boolean", python_typename=bool.__name__, From b3f1314963e0a1d6ea8ff957ac7c9b912d962a0b Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 12:50:39 -0500 Subject: [PATCH 09/39] Parameterize the scalar value test Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/unit/test_streamlit_panel.py | 34 ++++++++++++------------------ 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/tests/unit/test_streamlit_panel.py b/tests/unit/test_streamlit_panel.py index de121fd..db4dcc6 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -84,31 +84,25 @@ def test___unopened_panel___get_set_value___gets_value( assert panel.get_value(value_id) == string_value -def test___builtin_scalar_types___set_value___gets_same_value( +@pytest.mark.parametrize( + "value_payload", + [ + "firstname bunchanumbers", + 42, + 3.14, + True, + b"robotext", + ], +) +def test___builtin_scalar_type___set_value___gets_same_value( grpc_channel_for_fake_panel_service: grpc.Channel, + value_payload: object, ) -> None: """Test that set_value() and get_value() work for builtin scalar types.""" channel = grpc_channel_for_fake_panel_service panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) value_id = "test_id" - string_value = "test_value" - int_value = 42 - float_value = 3.14 - bool_value = True - bytes_value = b"robotext" - - panel.set_value(value_id, string_value) - assert panel.get_value(value_id) == string_value - - panel.set_value(value_id, int_value) - assert panel.get_value(value_id) == int_value - - panel.set_value(value_id, float_value) - assert panel.get_value(value_id) == float_value - - panel.set_value(value_id, bool_value) - assert panel.get_value(value_id) == bool_value + panel.set_value(value_id, value_payload) - panel.set_value(value_id, bytes_value) - assert panel.get_value(value_id) == bytes_value + assert panel.get_value(value_id) == value_payload From 4d73ca6dae84e5ce26f2c133f68c9154f2ffe33c Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 12:57:03 -0500 Subject: [PATCH 10/39] Add roundtrip tests for IntFlag, IntEnum, StrEnum Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/types.py | 27 +++++++++++++++++++++++++++ tests/unit/test_streamlit_panel.py | 4 ++++ 2 files changed, 31 insertions(+) create mode 100644 tests/types.py diff --git a/tests/types.py b/tests/types.py new file mode 100644 index 0000000..ec50738 --- /dev/null +++ b/tests/types.py @@ -0,0 +1,27 @@ +"""Types that support conversion to and from protobuf.""" + +import enum + + +class MyIntFlags(enum.IntFlag): + """Example of an IntFlag enum.""" + + VALUE1 = 1 + VALUE2 = 2 + VALUE4 = 4 + + +class MyIntEnum(enum.IntEnum): + """Example of an IntEnum enum.""" + + VALUE10 = 10 + VALUE20 = 20 + VALUE30 = 30 + + +class MyStrEnum(enum.StrEnum): + """Example of a StrEnum enum.""" + + VALUE1 = "value1" + VALUE2 = "value2" + VALUE3 = "value3" diff --git a/tests/unit/test_streamlit_panel.py b/tests/unit/test_streamlit_panel.py index db4dcc6..8a14b78 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -1,6 +1,7 @@ import grpc import pytest +import tests.types as test_types from nipanel._streamlit_panel import StreamlitPanel from tests.utils._fake_python_panel_service import FakePythonPanelService @@ -92,6 +93,9 @@ def test___unopened_panel___get_set_value___gets_value( 3.14, True, b"robotext", + test_types.MyIntFlags.VALUE1 | test_types.MyIntFlags.VALUE4, + test_types.MyIntEnum.VALUE20, + test_types.MyStrEnum.VALUE3, ], ) def test___builtin_scalar_type___set_value___gets_same_value( From 518dd2fdc294b5b0fb1b1cd40bd55c1e90201a8b Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 13:41:53 -0500 Subject: [PATCH 11/39] Add tests for mixin-enums and bare-enums Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/types.py | 30 ++++++++++++++++++++++++++++++ tests/unit/test_streamlit_panel.py | 22 ++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/tests/types.py b/tests/types.py index ec50738..f4458c5 100644 --- a/tests/types.py +++ b/tests/types.py @@ -19,9 +19,39 @@ class MyIntEnum(enum.IntEnum): VALUE30 = 30 +class MixinIntEnum(int, enum.Enum): + """Example of an IntEnum using a mixin.""" + VALUE11 = 11 + VALUE22 = 22 + VALUE33 = 33 + + class MyStrEnum(enum.StrEnum): """Example of a StrEnum enum.""" VALUE1 = "value1" VALUE2 = "value2" VALUE3 = "value3" + + +class MixinStrEnum(str, enum.Enum): + """Example of a StrEnum using a mixin.""" + VALUE11 = "value11" + VALUE22 = "value22" + VALUE33 = "value33" + + +class MyEnum(enum.Enum): + """Example of a simple enum.""" + + VALUE100 = 100 + VALUE200 = 200 + VALUE300 = 300 + + +class MyFlags(enum.Flag): + """Example of a simple flag.""" + + VALUE8 = 8 + VALUE16 = 16 + VALUE32 = 32 diff --git a/tests/unit/test_streamlit_panel.py b/tests/unit/test_streamlit_panel.py index 8a14b78..86e7dd7 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -96,6 +96,8 @@ def test___unopened_panel___get_set_value___gets_value( test_types.MyIntFlags.VALUE1 | test_types.MyIntFlags.VALUE4, test_types.MyIntEnum.VALUE20, test_types.MyStrEnum.VALUE3, + test_types.MixinIntEnum.VALUE33, + test_types.MixinStrEnum.VALUE11, ], ) def test___builtin_scalar_type___set_value___gets_same_value( @@ -110,3 +112,23 @@ def test___builtin_scalar_type___set_value___gets_same_value( panel.set_value(value_id, value_payload) assert panel.get_value(value_id) == value_payload + + +@pytest.mark.parametrize( + "value_payload", + [ + test_types.MyEnum.VALUE300, + test_types.MyFlags.VALUE8 | test_types.MyFlags.VALUE16, + ], +) +def test___unsupported_type___set_value___raises( + grpc_channel_for_fake_panel_service: grpc.Channel, + value_payload: object, +) -> None: + """Test that set_value() raises for unsupported types.""" + channel = grpc_channel_for_fake_panel_service + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + + value_id = "test_id" + with pytest.raises(TypeError): + panel.set_value(value_id, value_payload) From 345e89ac74c52abb693c2013c98332e7645c3488 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 13:44:45 -0500 Subject: [PATCH 12/39] Shorten typename imports Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 773e7b9..1bb61a5 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -6,18 +6,18 @@ from typing import Callable, NamedTuple, Union import google.protobuf.any_pb2 -import google.protobuf.wrappers_pb2 +from google.protobuf import wrappers_pb2 from typing_extensions import TypeAlias _logger = logging.getLogger(__name__) _builtin_protobuf_type: TypeAlias = Union[ - google.protobuf.wrappers_pb2.BoolValue, - google.protobuf.wrappers_pb2.BytesValue, - google.protobuf.wrappers_pb2.DoubleValue, - google.protobuf.wrappers_pb2.Int64Value, - google.protobuf.wrappers_pb2.StringValue, + wrappers_pb2.BoolValue, + wrappers_pb2.BytesValue, + wrappers_pb2.DoubleValue, + wrappers_pb2.Int64Value, + wrappers_pb2.StringValue, ] From 3a02372c9de330871deaa512c2bf46d922c0dddf Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 13:49:53 -0500 Subject: [PATCH 13/39] Rename fixture 'grpc_channel_for_fake_panel_service' to 'fake_panel_channel' Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/conftest.py | 7 +++--- tests/types.py | 2 ++ tests/unit/test_streamlit_panel.py | 35 ++++++++++++------------------ 3 files changed, 19 insertions(+), 25 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 580df51..c850b85 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -23,7 +23,7 @@ def fake_python_panel_service() -> Generator[FakePythonPanelService]: @pytest.fixture -def grpc_channel_for_fake_panel_service( +def fake_panel_channel( fake_python_panel_service: FakePythonPanelService, ) -> Generator[grpc.Channel]: """Fixture to get a channel to the FakePythonPanelService.""" @@ -35,8 +35,7 @@ def grpc_channel_for_fake_panel_service( @pytest.fixture def python_panel_service_stub( - grpc_channel_for_fake_panel_service: grpc.Channel, + fake_panel_channel: grpc.Channel, ) -> Generator[PythonPanelServiceStub]: """Fixture to get a PythonPanelServiceStub, attached to a FakePythonPanelService.""" - channel = grpc_channel_for_fake_panel_service - yield PythonPanelServiceStub(channel) + yield PythonPanelServiceStub(fake_panel_channel) diff --git a/tests/types.py b/tests/types.py index f4458c5..7503953 100644 --- a/tests/types.py +++ b/tests/types.py @@ -21,6 +21,7 @@ class MyIntEnum(enum.IntEnum): class MixinIntEnum(int, enum.Enum): """Example of an IntEnum using a mixin.""" + VALUE11 = 11 VALUE22 = 22 VALUE33 = 33 @@ -36,6 +37,7 @@ class MyStrEnum(enum.StrEnum): class MixinStrEnum(str, enum.Enum): """Example of a StrEnum using a mixin.""" + VALUE11 = "value11" VALUE22 = "value22" VALUE33 = "value33" diff --git a/tests/unit/test_streamlit_panel.py b/tests/unit/test_streamlit_panel.py index 86e7dd7..d85de8f 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -13,10 +13,9 @@ def test___panel___has_panel_id_and_panel_uri() -> None: def test___opened_panel___set_value___gets_same_value( - grpc_channel_for_fake_panel_service: grpc.Channel, + fake_panel_channel: grpc.Channel, ) -> None: - channel = grpc_channel_for_fake_panel_service - panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) panel.open_panel() value_id = "test_id" @@ -28,14 +27,13 @@ def test___opened_panel___set_value___gets_same_value( def test___first_open_panel_fails___open_panel___gets_value( fake_python_panel_service: FakePythonPanelService, - grpc_channel_for_fake_panel_service: grpc.Channel, + fake_panel_channel: grpc.Channel, ) -> None: """Test that panel.open_panel() will automatically retry once.""" - channel = grpc_channel_for_fake_panel_service service = fake_python_panel_service # Simulate a failure on the first attempt service.servicer.fail_next_open_panel() - panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) panel.open_panel() @@ -46,11 +44,10 @@ def test___first_open_panel_fails___open_panel___gets_value( def test___unopened_panel___set_value___sets_value( - grpc_channel_for_fake_panel_service: grpc.Channel, + fake_panel_channel: grpc.Channel, ) -> None: """Test that set_value() succeeds before the user opens the panel.""" - channel = grpc_channel_for_fake_panel_service - panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) value_id = "test_id" string_value = "test_value" @@ -60,11 +57,10 @@ def test___unopened_panel___set_value___sets_value( def test___unopened_panel___get_unset_value___raises_exception( - grpc_channel_for_fake_panel_service: grpc.Channel, + fake_panel_channel: grpc.Channel, ) -> None: """Test that get_value() raises an exception for an unset value.""" - channel = grpc_channel_for_fake_panel_service - panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) value_id = "test_id" with pytest.raises(grpc.RpcError): @@ -72,11 +68,10 @@ def test___unopened_panel___get_unset_value___raises_exception( def test___unopened_panel___get_set_value___gets_value( - grpc_channel_for_fake_panel_service: grpc.Channel, + fake_panel_channel: grpc.Channel, ) -> None: """Test that get_value() succeeds for a set value before the user opens the panel.""" - channel = grpc_channel_for_fake_panel_service - panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) value_id = "test_id" string_value = "test_value" @@ -101,12 +96,11 @@ def test___unopened_panel___get_set_value___gets_value( ], ) def test___builtin_scalar_type___set_value___gets_same_value( - grpc_channel_for_fake_panel_service: grpc.Channel, + fake_panel_channel: grpc.Channel, value_payload: object, ) -> None: """Test that set_value() and get_value() work for builtin scalar types.""" - channel = grpc_channel_for_fake_panel_service - panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) value_id = "test_id" panel.set_value(value_id, value_payload) @@ -122,12 +116,11 @@ def test___builtin_scalar_type___set_value___gets_same_value( ], ) def test___unsupported_type___set_value___raises( - grpc_channel_for_fake_panel_service: grpc.Channel, + fake_panel_channel: grpc.Channel, value_payload: object, ) -> None: """Test that set_value() raises for unsupported types.""" - channel = grpc_channel_for_fake_panel_service - panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=channel) + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) value_id = "test_id" with pytest.raises(TypeError): From ca9b8a306ab8ceb7b64967313f3b9ed3959bd08d Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 14:11:53 -0500 Subject: [PATCH 14/39] Use grcpio's logging thread pool Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index c850b85..9205fc1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,10 +1,10 @@ """Fixtures for testing.""" from collections.abc import Generator -from concurrent import futures import grpc import pytest +from grpc.framework.foundation import logging_pool # type: ignore # types-grpcio does not cover this yet from ni.pythonpanel.v1.python_panel_service_pb2_grpc import ( PythonPanelServiceStub, ) @@ -15,7 +15,7 @@ @pytest.fixture def fake_python_panel_service() -> Generator[FakePythonPanelService]: """Fixture to create a FakePythonPanelServicer for testing.""" - with futures.ThreadPoolExecutor(max_workers=10) as thread_pool: + with logging_pool.pool(max_workers=10) as thread_pool: service = FakePythonPanelService() service.start(thread_pool) yield service From 91ee96b8364bb449a5c8cc77094d8af2418d56b8 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 14:15:47 -0500 Subject: [PATCH 15/39] Remove static storage for the fake servicer Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/utils/_fake_python_panel_servicer.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/utils/_fake_python_panel_servicer.py b/tests/utils/_fake_python_panel_servicer.py index 7427400..9e32a84 100644 --- a/tests/utils/_fake_python_panel_servicer.py +++ b/tests/utils/_fake_python_panel_servicer.py @@ -16,8 +16,10 @@ class FakePythonPanelServicer(PythonPanelServiceServicer): """Fake implementation of the PythonPanelServicer for testing.""" - _values = {"test_value": any_pb2.Any()} - _fail_next_open_panel = False + def __init__(self) -> None: + """Initialize the fake PythonPanelServicer.""" + self._values = {"test_value": any_pb2.Any()} + self._fail_next_open_panel = False def OpenPanel(self, request: OpenPanelRequest, context: Any) -> OpenPanelResponse: # noqa: N802 """Trivial implementation for testing.""" From 63296e1ea5aeeb50eadb9d9138482d7f7b70251d Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 14:24:33 -0500 Subject: [PATCH 16/39] Remove class members from the fake service Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/utils/_fake_python_panel_service.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/utils/_fake_python_panel_service.py b/tests/utils/_fake_python_panel_service.py index 43e8f5a..547f0db 100644 --- a/tests/utils/_fake_python_panel_service.py +++ b/tests/utils/_fake_python_panel_service.py @@ -11,12 +11,10 @@ class FakePythonPanelService: """Encapsulates a fake PythonPanelService with a gRPC server for testing.""" - _server: grpc.Server - _port: int - _servicer: FakePythonPanelServicer - def __init__(self) -> None: """Initialize the fake PythonPanelService.""" + self._server: grpc.Server + self._port: int self._servicer = FakePythonPanelServicer() def start(self, thread_pool: futures.ThreadPoolExecutor) -> None: From 5969e92f2acf3fd0cf70aa45a5426fc3b0d98e5a Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 14:30:55 -0500 Subject: [PATCH 17/39] Simplify class and storage for ConvertibleType Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 1bb61a5..405109d 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -24,9 +24,6 @@ class ConvertibleType(NamedTuple): """A Python type that can be converted to and from a protobuf Any.""" - name: str - """The human name of the type.""" - python_typename: str """The Python name for the type.""" @@ -37,44 +34,39 @@ class ConvertibleType(NamedTuple): """A callable that can be used to create an instance of the protobuf type.""" -_CONVERTIBLE_TYPES = { - "bool": ConvertibleType( - name="Boolean", +_CONVERTIBLE_TYPES = [ + ConvertibleType( python_typename=bool.__name__, protobuf_typename=google.protobuf.wrappers_pb2.BoolValue.DESCRIPTOR.full_name, protobuf_initializer=google.protobuf.wrappers_pb2.BoolValue, ), - "bytes": ConvertibleType( - name="Bytes", + ConvertibleType( python_typename=bytes.__name__, protobuf_typename=google.protobuf.wrappers_pb2.BytesValue.DESCRIPTOR.full_name, protobuf_initializer=google.protobuf.wrappers_pb2.BytesValue, ), - "float": ConvertibleType( - name="Float", + ConvertibleType( python_typename=float.__name__, protobuf_typename=google.protobuf.wrappers_pb2.DoubleValue.DESCRIPTOR.full_name, protobuf_initializer=google.protobuf.wrappers_pb2.DoubleValue, ), - "int": ConvertibleType( - name="Integer", + ConvertibleType( python_typename=int.__name__, protobuf_typename=google.protobuf.wrappers_pb2.Int64Value.DESCRIPTOR.full_name, protobuf_initializer=google.protobuf.wrappers_pb2.Int64Value, ), - "str": ConvertibleType( - name="String", + ConvertibleType( python_typename=str.__name__, protobuf_typename=google.protobuf.wrappers_pb2.StringValue.DESCRIPTOR.full_name, protobuf_initializer=google.protobuf.wrappers_pb2.StringValue, ), -} +] def to_any(python_value: object) -> google.protobuf.any_pb2.Any: """Convert a Python object to a protobuf Any.""" protobuf_wrapper_for_type = { - entry.python_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES.values() + entry.python_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES } supported_types = set(protobuf_wrapper_for_type.keys()) underlying_parents = [ @@ -100,7 +92,7 @@ def to_any(python_value: object) -> google.protobuf.any_pb2.Any: def from_any(protobuf_any: google.protobuf.any_pb2.Any) -> object: """Convert a protobuf Any to a Python object.""" protobuf_wrapper_for_type = { - entry.protobuf_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES.values() + entry.protobuf_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES } if not isinstance(protobuf_any, google.protobuf.any_pb2.Any): raise ValueError(f"Unexpected type: {type(protobuf_any)}") From 3fb2b9be14be2b94e7221f20f940904157d31a0e Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 15:17:41 -0500 Subject: [PATCH 18/39] Use pyproject.toml to suppress type checker Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- pyproject.toml | 1 + tests/conftest.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index dd0056e..20f58ac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,6 +67,7 @@ strict = true module = [ # https://github.com/ni/hightime/issues/4 - Add type annotations "hightime.*", + "grpc.framework.foundation.*", ] ignore_missing_imports = true diff --git a/tests/conftest.py b/tests/conftest.py index 9205fc1..1cb4284 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,7 +4,7 @@ import grpc import pytest -from grpc.framework.foundation import logging_pool # type: ignore # types-grpcio does not cover this yet +from grpc.framework.foundation import logging_pool from ni.pythonpanel.v1.python_panel_service_pb2_grpc import ( PythonPanelServiceStub, ) From 62f808c0d818fc7646c554d68352041229e20621 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 15:18:39 -0500 Subject: [PATCH 19/39] Clarify docstring Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/types.py b/tests/types.py index 7503953..c0b9738 100644 --- a/tests/types.py +++ b/tests/types.py @@ -1,4 +1,4 @@ -"""Types that support conversion to and from protobuf.""" +"""Types that exercise conversion to and from protobuf.""" import enum From 9a5ae3c5c44e7e2d03384c4c789167d889132fec Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 15:26:24 -0500 Subject: [PATCH 20/39] Do not use 'as' for imports because it confuses tooling Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/utils/_fake_python_panel_servicer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils/_fake_python_panel_servicer.py b/tests/utils/_fake_python_panel_servicer.py index 9e32a84..920d3dd 100644 --- a/tests/utils/_fake_python_panel_servicer.py +++ b/tests/utils/_fake_python_panel_servicer.py @@ -1,7 +1,7 @@ from typing import Any -import google.protobuf.any_pb2 as any_pb2 import grpc +from google.protobuf import any_pb2 from ni.pythonpanel.v1.python_panel_service_pb2 import ( OpenPanelRequest, OpenPanelResponse, From eabcb0f9a086ac91f8f311335c2ec325bc9ad487 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 15:45:38 -0500 Subject: [PATCH 21/39] Make types compatible with Python 3.9 and 3.10 Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- examples/placeholder.py | 4 ++-- tests/types.py | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/examples/placeholder.py b/examples/placeholder.py index e9b1216..53ad9a3 100644 --- a/examples/placeholder.py +++ b/examples/placeholder.py @@ -21,8 +21,8 @@ class MyIntEnum(enum.IntEnum): VALUE30 = 30 -class MyStrEnum(enum.StrEnum): - """Example of a StrEnum enum.""" +class MyStrEnum(str, enum.Enum): + """Example of a mixin string enum.""" VALUE1 = "value1" VALUE2 = "value2" diff --git a/tests/types.py b/tests/types.py index c0b9738..0d17f9b 100644 --- a/tests/types.py +++ b/tests/types.py @@ -1,6 +1,16 @@ """Types that exercise conversion to and from protobuf.""" import enum +import sys + +if sys.version_info >= (3, 11): + from enum import StrEnum +else: + + class StrEnum(str, enum.Enum): + """Example of a mixin string enum.""" + + pass class MyIntFlags(enum.IntFlag): @@ -27,7 +37,7 @@ class MixinIntEnum(int, enum.Enum): VALUE33 = 33 -class MyStrEnum(enum.StrEnum): +class MyStrEnum(StrEnum): """Example of a StrEnum enum.""" VALUE1 = "value1" From 6457e4196430133c49bc1ca963f62fb2a1b75d85 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 16:57:26 -0500 Subject: [PATCH 22/39] Remove redundant namespacing and front-load building the type conversion dicts Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 56 ++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 405109d..8195f93 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -5,8 +5,7 @@ import logging from typing import Callable, NamedTuple, Union -import google.protobuf.any_pb2 -from google.protobuf import wrappers_pb2 +from google.protobuf import any_pb2, wrappers_pb2 from typing_extensions import TypeAlias _logger = logging.getLogger(__name__) @@ -37,70 +36,73 @@ class ConvertibleType(NamedTuple): _CONVERTIBLE_TYPES = [ ConvertibleType( python_typename=bool.__name__, - protobuf_typename=google.protobuf.wrappers_pb2.BoolValue.DESCRIPTOR.full_name, - protobuf_initializer=google.protobuf.wrappers_pb2.BoolValue, + protobuf_typename=wrappers_pb2.BoolValue.DESCRIPTOR.full_name, + protobuf_initializer=wrappers_pb2.BoolValue, ), ConvertibleType( python_typename=bytes.__name__, - protobuf_typename=google.protobuf.wrappers_pb2.BytesValue.DESCRIPTOR.full_name, - protobuf_initializer=google.protobuf.wrappers_pb2.BytesValue, + protobuf_typename=wrappers_pb2.BytesValue.DESCRIPTOR.full_name, + protobuf_initializer=wrappers_pb2.BytesValue, ), ConvertibleType( python_typename=float.__name__, - protobuf_typename=google.protobuf.wrappers_pb2.DoubleValue.DESCRIPTOR.full_name, - protobuf_initializer=google.protobuf.wrappers_pb2.DoubleValue, + protobuf_typename=wrappers_pb2.DoubleValue.DESCRIPTOR.full_name, + protobuf_initializer=wrappers_pb2.DoubleValue, ), ConvertibleType( python_typename=int.__name__, - protobuf_typename=google.protobuf.wrappers_pb2.Int64Value.DESCRIPTOR.full_name, - protobuf_initializer=google.protobuf.wrappers_pb2.Int64Value, + protobuf_typename=wrappers_pb2.Int64Value.DESCRIPTOR.full_name, + protobuf_initializer=wrappers_pb2.Int64Value, ), ConvertibleType( python_typename=str.__name__, - protobuf_typename=google.protobuf.wrappers_pb2.StringValue.DESCRIPTOR.full_name, - protobuf_initializer=google.protobuf.wrappers_pb2.StringValue, + protobuf_typename=wrappers_pb2.StringValue.DESCRIPTOR.full_name, + protobuf_initializer=wrappers_pb2.StringValue, ), ] +_PROTOBUF_WRAPPER_FOR_PYTHON_TYPE = { + entry.python_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES +} -def to_any(python_value: object) -> google.protobuf.any_pb2.Any: +_PROTOBUF_WRAPPER_FOR_GRPC_TYPE = { + entry.protobuf_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES +} + +_SUPPORTED_PYTHON_TYPES = _PROTOBUF_WRAPPER_FOR_PYTHON_TYPE.keys() + + +def to_any(python_value: object) -> any_pb2.Any: """Convert a Python object to a protobuf Any.""" - protobuf_wrapper_for_type = { - entry.python_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES - } - supported_types = set(protobuf_wrapper_for_type.keys()) underlying_parents = [ parent.__name__ for parent in type(python_value).mro() # This covers enum.IntEnum and similar ] best_matching_type = next( - (parent for parent in underlying_parents if parent in supported_types), None + (parent for parent in underlying_parents if parent in _SUPPORTED_PYTHON_TYPES), None ) if not best_matching_type: raise TypeError( - f"Unsupported type: {type(python_value)} with parents {underlying_parents}. Supported types are: {supported_types}" + f"Unsupported type: {type(python_value)} with parents {underlying_parents}. Supported types are: {_SUPPORTED_PYTHON_TYPES}" ) _logger.debug(f"Best matching type for '{repr(python_value)}' resolved to {best_matching_type}") - the_any = google.protobuf.any_pb2.Any() - wrapped_value = protobuf_wrapper_for_type[best_matching_type](value=python_value) + the_any = any_pb2.Any() + wrapped_value = _PROTOBUF_WRAPPER_FOR_PYTHON_TYPE[best_matching_type](value=python_value) the_any.Pack(wrapped_value) return the_any -def from_any(protobuf_any: google.protobuf.any_pb2.Any) -> object: +def from_any(protobuf_any: any_pb2.Any) -> object: """Convert a protobuf Any to a Python object.""" - protobuf_wrapper_for_type = { - entry.protobuf_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES - } - if not isinstance(protobuf_any, google.protobuf.any_pb2.Any): + if not isinstance(protobuf_any, any_pb2.Any): raise ValueError(f"Unexpected type: {type(protobuf_any)}") underlying_typename = protobuf_any.TypeName() _logger.debug(f"Unpacking type '{underlying_typename}'") - protobuf_wrapper = protobuf_wrapper_for_type[underlying_typename]() + protobuf_wrapper = _PROTOBUF_WRAPPER_FOR_GRPC_TYPE[underlying_typename]() did_unpack = protobuf_any.Unpack(protobuf_wrapper) if not did_unpack: raise ValueError(f"Failed to unpack Any with underlying type '{underlying_typename}'") From fde87f6ee51fc0c8a5864f09c9e40f781b4c4522 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Thu, 15 May 2025 17:05:03 -0500 Subject: [PATCH 23/39] Use the Python types rather than typenames for to_any Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 8195f93..2e57223 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -23,8 +23,8 @@ class ConvertibleType(NamedTuple): """A Python type that can be converted to and from a protobuf Any.""" - python_typename: str - """The Python name for the type.""" + python_type: type + """The Python type.""" protobuf_typename: str """The protobuf name for the type.""" @@ -35,34 +35,34 @@ class ConvertibleType(NamedTuple): _CONVERTIBLE_TYPES = [ ConvertibleType( - python_typename=bool.__name__, + python_type=bool, protobuf_typename=wrappers_pb2.BoolValue.DESCRIPTOR.full_name, protobuf_initializer=wrappers_pb2.BoolValue, ), ConvertibleType( - python_typename=bytes.__name__, + python_type=bytes, protobuf_typename=wrappers_pb2.BytesValue.DESCRIPTOR.full_name, protobuf_initializer=wrappers_pb2.BytesValue, ), ConvertibleType( - python_typename=float.__name__, + python_type=float, protobuf_typename=wrappers_pb2.DoubleValue.DESCRIPTOR.full_name, protobuf_initializer=wrappers_pb2.DoubleValue, ), ConvertibleType( - python_typename=int.__name__, + python_type=int, protobuf_typename=wrappers_pb2.Int64Value.DESCRIPTOR.full_name, protobuf_initializer=wrappers_pb2.Int64Value, ), ConvertibleType( - python_typename=str.__name__, + python_type=str, protobuf_typename=wrappers_pb2.StringValue.DESCRIPTOR.full_name, protobuf_initializer=wrappers_pb2.StringValue, ), ] _PROTOBUF_WRAPPER_FOR_PYTHON_TYPE = { - entry.python_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES + entry.python_type: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES } _PROTOBUF_WRAPPER_FOR_GRPC_TYPE = { @@ -74,10 +74,7 @@ class ConvertibleType(NamedTuple): def to_any(python_value: object) -> any_pb2.Any: """Convert a Python object to a protobuf Any.""" - underlying_parents = [ - parent.__name__ - for parent in type(python_value).mro() # This covers enum.IntEnum and similar - ] + underlying_parents = type(python_value).mro() # This covers enum.IntEnum and similar best_matching_type = next( (parent for parent in underlying_parents if parent in _SUPPORTED_PYTHON_TYPES), None From 905e315c8fc9af132544b1ea66e3fd4e9ad205c7 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Fri, 16 May 2025 10:38:08 -0500 Subject: [PATCH 24/39] Shortest path to custom initializers Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 66 +++++++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 18 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 2e57223..ddf333c 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -6,20 +6,12 @@ from typing import Callable, NamedTuple, Union from google.protobuf import any_pb2, wrappers_pb2 +from google.protobuf.message import Message from typing_extensions import TypeAlias _logger = logging.getLogger(__name__) -_builtin_protobuf_type: TypeAlias = Union[ - wrappers_pb2.BoolValue, - wrappers_pb2.BytesValue, - wrappers_pb2.DoubleValue, - wrappers_pb2.Int64Value, - wrappers_pb2.StringValue, -] - - class ConvertibleType(NamedTuple): """A Python type that can be converted to and from a protobuf Any.""" @@ -29,35 +21,73 @@ class ConvertibleType(NamedTuple): protobuf_typename: str """The protobuf name for the type.""" - protobuf_initializer: Callable[..., _builtin_protobuf_type] - """A callable that can be used to create an instance of the protobuf type.""" + protobuf_message: type + """The protobuf message for the type.""" + + protobuf_initializer: Callable[[object], Message] + """A callable that creates an instance of the protobuf type from a Python object.""" + + +def _bool_to_protobuf(value: object) -> wrappers_pb2.BoolValue: + """Convert a bool to a protobuf BoolValue.""" + assert isinstance(value, bool), f"Expected bool, got {type(value)}" + return wrappers_pb2.BoolValue(value=value) + + +def _bytes_to_protobuf(value: object) -> wrappers_pb2.BytesValue: + """Convert bytes to a protobuf BytesValue.""" + assert isinstance(value, bytes), f"Expected bytes, got {type(value)}" + return wrappers_pb2.BytesValue(value=value) + + +def _float_to_protobuf(value: object) -> wrappers_pb2.DoubleValue: + """Convert a float to a protobuf DoubleValue.""" + assert isinstance(value, float), f"Expected float, got {type(value)}" + return wrappers_pb2.DoubleValue(value=value) + + +def _int_to_protobuf(value: object) -> wrappers_pb2.Int64Value: + """Convert an int to a protobuf Int64Value.""" + assert isinstance(value, int), f"Expected int, got {type(value)}" + return wrappers_pb2.Int64Value(value=value) + + +def _str_to_protobuf(value: object) -> wrappers_pb2.StringValue: + """Convert a str to a protobuf StringValue.""" + assert isinstance(value, str), f"Expected str, got {type(value)}" + return wrappers_pb2.StringValue(value=value) _CONVERTIBLE_TYPES = [ ConvertibleType( python_type=bool, protobuf_typename=wrappers_pb2.BoolValue.DESCRIPTOR.full_name, - protobuf_initializer=wrappers_pb2.BoolValue, + protobuf_message=wrappers_pb2.BoolValue, + protobuf_initializer=_bool_to_protobuf, ), ConvertibleType( python_type=bytes, protobuf_typename=wrappers_pb2.BytesValue.DESCRIPTOR.full_name, - protobuf_initializer=wrappers_pb2.BytesValue, + protobuf_message=wrappers_pb2.BytesValue, + protobuf_initializer=_bytes_to_protobuf, ), ConvertibleType( python_type=float, protobuf_typename=wrappers_pb2.DoubleValue.DESCRIPTOR.full_name, - protobuf_initializer=wrappers_pb2.DoubleValue, + protobuf_message=wrappers_pb2.DoubleValue, + protobuf_initializer=_float_to_protobuf, ), ConvertibleType( python_type=int, protobuf_typename=wrappers_pb2.Int64Value.DESCRIPTOR.full_name, - protobuf_initializer=wrappers_pb2.Int64Value, + protobuf_message=wrappers_pb2.Int64Value, + protobuf_initializer=_int_to_protobuf, ), ConvertibleType( python_type=str, protobuf_typename=wrappers_pb2.StringValue.DESCRIPTOR.full_name, - protobuf_initializer=wrappers_pb2.StringValue, + protobuf_message=wrappers_pb2.StringValue, + protobuf_initializer=_str_to_protobuf, ), ] @@ -66,7 +96,7 @@ class ConvertibleType(NamedTuple): } _PROTOBUF_WRAPPER_FOR_GRPC_TYPE = { - entry.protobuf_typename: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES + entry.protobuf_typename: entry.protobuf_message for entry in _CONVERTIBLE_TYPES } _SUPPORTED_PYTHON_TYPES = _PROTOBUF_WRAPPER_FOR_PYTHON_TYPE.keys() @@ -86,7 +116,7 @@ def to_any(python_value: object) -> any_pb2.Any: _logger.debug(f"Best matching type for '{repr(python_value)}' resolved to {best_matching_type}") the_any = any_pb2.Any() - wrapped_value = _PROTOBUF_WRAPPER_FOR_PYTHON_TYPE[best_matching_type](value=python_value) + wrapped_value = _PROTOBUF_WRAPPER_FOR_PYTHON_TYPE[best_matching_type](python_value) the_any.Pack(wrapped_value) return the_any From b73b7b0a7bd21863ee47a87aa6db5b209d7cef45 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Fri, 16 May 2025 11:27:07 -0500 Subject: [PATCH 25/39] Use a Protocol to allow converters to have custom conversions Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 187 +++++++++++++++++++++++-------------- 1 file changed, 116 insertions(+), 71 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index ddf333c..cf37cc4 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -3,103 +3,146 @@ from __future__ import annotations import logging -from typing import Callable, NamedTuple, Union +from typing import Protocol from google.protobuf import any_pb2, wrappers_pb2 from google.protobuf.message import Message -from typing_extensions import TypeAlias _logger = logging.getLogger(__name__) -class ConvertibleType(NamedTuple): - """A Python type that can be converted to and from a protobuf Any.""" +class Converter(Protocol): + """A class that defines how to convert between Python and protobuf types.""" - python_type: type - """The Python type.""" + @property + def python_type(self) -> type: + """The Python type that this converter handles.""" - protobuf_typename: str - """The protobuf name for the type.""" + @property + def protobuf_message(self) -> type: + """The protobuf message for the type.""" - protobuf_message: type - """The protobuf message for the type.""" + def to_protobuf(self, value: object) -> Message: + """Convert a Python object to a protobuf message.""" - protobuf_initializer: Callable[[object], Message] - """A callable that creates an instance of the protobuf type from a Python object.""" + @property + def protobuf_typename(self) -> str: + """The protobuf name for the type.""" + return self.protobuf_message.DESCRIPTOR.full_name -def _bool_to_protobuf(value: object) -> wrappers_pb2.BoolValue: - """Convert a bool to a protobuf BoolValue.""" - assert isinstance(value, bool), f"Expected bool, got {type(value)}" - return wrappers_pb2.BoolValue(value=value) +class BoolConverter(Converter): + """A converter for bool types.""" + @property + def python_type(self) -> type: + """The Python type that this converter handles.""" + return bool -def _bytes_to_protobuf(value: object) -> wrappers_pb2.BytesValue: - """Convert bytes to a protobuf BytesValue.""" - assert isinstance(value, bytes), f"Expected bytes, got {type(value)}" - return wrappers_pb2.BytesValue(value=value) + @property + def protobuf_message(self) -> type: + """The protobuf message for the type.""" + return wrappers_pb2.BoolValue + def to_protobuf(self, value: object) -> wrappers_pb2.BoolValue: + """Convert a bool to a protobuf BoolValue.""" + assert isinstance(value, bool), f"Expected bool, got {type(value)}" + return self.protobuf_message(value=value) -def _float_to_protobuf(value: object) -> wrappers_pb2.DoubleValue: - """Convert a float to a protobuf DoubleValue.""" - assert isinstance(value, float), f"Expected float, got {type(value)}" - return wrappers_pb2.DoubleValue(value=value) +class BytesConverter(Converter): + """A converter for bytes types.""" -def _int_to_protobuf(value: object) -> wrappers_pb2.Int64Value: - """Convert an int to a protobuf Int64Value.""" - assert isinstance(value, int), f"Expected int, got {type(value)}" - return wrappers_pb2.Int64Value(value=value) + @property + def python_type(self) -> type: + """The Python type that this converter handles.""" + return bytes + @property + def protobuf_message(self) -> type: + """The protobuf message for the type.""" + return wrappers_pb2.BytesValue -def _str_to_protobuf(value: object) -> wrappers_pb2.StringValue: - """Convert a str to a protobuf StringValue.""" - assert isinstance(value, str), f"Expected str, got {type(value)}" - return wrappers_pb2.StringValue(value=value) + def to_protobuf(self, value: object) -> wrappers_pb2.BytesValue: + """Convert bytes to a protobuf BytesValue.""" + assert isinstance(value, bytes), f"Expected bytes, got {type(value)}" + return self.protobuf_message(value=value) + + +class FloatConverter(Converter): + """A converter for float types.""" + + @property + def python_type(self) -> type: + """The Python type that this converter handles.""" + return float + + @property + def protobuf_message(self) -> type: + """The protobuf message for the type.""" + return wrappers_pb2.DoubleValue + + def to_protobuf(self, value: object) -> wrappers_pb2.DoubleValue: + """Convert a float to a protobuf DoubleValue.""" + assert isinstance(value, float), f"Expected float, got {type(value)}" + return self.protobuf_message(value=value) + + +class IntConverter(Converter): + """A converter for int types.""" + + @property + def python_type(self) -> type: + """The Python type that this converter handles.""" + return int + + @property + def protobuf_message(self) -> type: + """The protobuf message for the type.""" + return wrappers_pb2.Int64Value + + def to_protobuf(self, value: object) -> wrappers_pb2.Int64Value: + """Convert an int to a protobuf Int64Value.""" + assert isinstance(value, int), f"Expected int, got {type(value)}" + return self.protobuf_message(value=value) + + +class StrConverter(Converter): + """A converter for str types.""" + + @property + def python_type(self) -> type: + """The Python type that this converter handles.""" + return str + + @property + def protobuf_message(self) -> type: + """The protobuf message for the type.""" + return wrappers_pb2.StringValue + + def to_protobuf(self, value: object) -> wrappers_pb2.StringValue: + """Convert a str to a protobuf StringValue.""" + assert isinstance(value, str), f"Expected str, got {type(value)}" + return self.protobuf_message(value=value) _CONVERTIBLE_TYPES = [ - ConvertibleType( - python_type=bool, - protobuf_typename=wrappers_pb2.BoolValue.DESCRIPTOR.full_name, - protobuf_message=wrappers_pb2.BoolValue, - protobuf_initializer=_bool_to_protobuf, - ), - ConvertibleType( - python_type=bytes, - protobuf_typename=wrappers_pb2.BytesValue.DESCRIPTOR.full_name, - protobuf_message=wrappers_pb2.BytesValue, - protobuf_initializer=_bytes_to_protobuf, - ), - ConvertibleType( - python_type=float, - protobuf_typename=wrappers_pb2.DoubleValue.DESCRIPTOR.full_name, - protobuf_message=wrappers_pb2.DoubleValue, - protobuf_initializer=_float_to_protobuf, - ), - ConvertibleType( - python_type=int, - protobuf_typename=wrappers_pb2.Int64Value.DESCRIPTOR.full_name, - protobuf_message=wrappers_pb2.Int64Value, - protobuf_initializer=_int_to_protobuf, - ), - ConvertibleType( - python_type=str, - protobuf_typename=wrappers_pb2.StringValue.DESCRIPTOR.full_name, - protobuf_message=wrappers_pb2.StringValue, - protobuf_initializer=_str_to_protobuf, - ), + BoolConverter(), + BytesConverter(), + FloatConverter(), + IntConverter(), + StrConverter(), ] -_PROTOBUF_WRAPPER_FOR_PYTHON_TYPE = { - entry.python_type: entry.protobuf_initializer for entry in _CONVERTIBLE_TYPES +_CONVERTER_FOR_PYTHON_TYPE = { + entry.python_type: entry for entry in _CONVERTIBLE_TYPES } -_PROTOBUF_WRAPPER_FOR_GRPC_TYPE = { - entry.protobuf_typename: entry.protobuf_message for entry in _CONVERTIBLE_TYPES +_CONVERTER_FOR_GRPC_TYPE = { + entry.protobuf_typename: entry for entry in _CONVERTIBLE_TYPES } -_SUPPORTED_PYTHON_TYPES = _PROTOBUF_WRAPPER_FOR_PYTHON_TYPE.keys() +_SUPPORTED_PYTHON_TYPES = _CONVERTER_FOR_PYTHON_TYPE.keys() def to_any(python_value: object) -> any_pb2.Any: @@ -116,7 +159,8 @@ def to_any(python_value: object) -> any_pb2.Any: _logger.debug(f"Best matching type for '{repr(python_value)}' resolved to {best_matching_type}") the_any = any_pb2.Any() - wrapped_value = _PROTOBUF_WRAPPER_FOR_PYTHON_TYPE[best_matching_type](python_value) + converter = _CONVERTER_FOR_PYTHON_TYPE[best_matching_type] + wrapped_value = converter.to_protobuf(python_value) the_any.Pack(wrapped_value) return the_any @@ -129,9 +173,10 @@ def from_any(protobuf_any: any_pb2.Any) -> object: underlying_typename = protobuf_any.TypeName() _logger.debug(f"Unpacking type '{underlying_typename}'") - protobuf_wrapper = _PROTOBUF_WRAPPER_FOR_GRPC_TYPE[underlying_typename]() - did_unpack = protobuf_any.Unpack(protobuf_wrapper) + converter = _CONVERTER_FOR_GRPC_TYPE[underlying_typename] + protobuf_message = converter.protobuf_message() + did_unpack = protobuf_any.Unpack(protobuf_message) if not did_unpack: raise ValueError(f"Failed to unpack Any with underlying type '{underlying_typename}'") - return protobuf_wrapper.value + return protobuf_message.value From e07a2c058fd8dc6b563a5d07c7286bf575a16cac Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Fri, 16 May 2025 11:28:09 -0500 Subject: [PATCH 26/39] Apply formatter Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index cf37cc4..4eba587 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -134,13 +134,9 @@ def to_protobuf(self, value: object) -> wrappers_pb2.StringValue: StrConverter(), ] -_CONVERTER_FOR_PYTHON_TYPE = { - entry.python_type: entry for entry in _CONVERTIBLE_TYPES -} +_CONVERTER_FOR_PYTHON_TYPE = {entry.python_type: entry for entry in _CONVERTIBLE_TYPES} -_CONVERTER_FOR_GRPC_TYPE = { - entry.protobuf_typename: entry for entry in _CONVERTIBLE_TYPES -} +_CONVERTER_FOR_GRPC_TYPE = {entry.protobuf_typename: entry for entry in _CONVERTIBLE_TYPES} _SUPPORTED_PYTHON_TYPES = _CONVERTER_FOR_PYTHON_TYPE.keys() From 32855055733c18bd0187aa9f25d7975848365bf1 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Fri, 16 May 2025 11:40:44 -0500 Subject: [PATCH 27/39] Make protobuf_message return a Message so the default implementation can access DESCRIPTOR Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 4eba587..ded2f1a 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -19,11 +19,11 @@ def python_type(self) -> type: """The Python type that this converter handles.""" @property - def protobuf_message(self) -> type: - """The protobuf message for the type.""" + def protobuf_message(self) -> Message: + """The matching protobuf message for the Python type.""" def to_protobuf(self, value: object) -> Message: - """Convert a Python object to a protobuf message.""" + """Convert the Python object to its type-specific protobuf message.""" @property def protobuf_typename(self) -> str: @@ -40,8 +40,8 @@ def python_type(self) -> type: return bool @property - def protobuf_message(self) -> type: - """The protobuf message for the type.""" + def protobuf_message(self) -> Message: + """The matching protobuf message for the Python type.""" return wrappers_pb2.BoolValue def to_protobuf(self, value: object) -> wrappers_pb2.BoolValue: @@ -59,8 +59,8 @@ def python_type(self) -> type: return bytes @property - def protobuf_message(self) -> type: - """The protobuf message for the type.""" + def protobuf_message(self) -> Message: + """The matching protobuf message for the Python type.""" return wrappers_pb2.BytesValue def to_protobuf(self, value: object) -> wrappers_pb2.BytesValue: @@ -78,8 +78,8 @@ def python_type(self) -> type: return float @property - def protobuf_message(self) -> type: - """The protobuf message for the type.""" + def protobuf_message(self) -> Message: + """The matching protobuf message for the Python type.""" return wrappers_pb2.DoubleValue def to_protobuf(self, value: object) -> wrappers_pb2.DoubleValue: @@ -97,8 +97,8 @@ def python_type(self) -> type: return int @property - def protobuf_message(self) -> type: - """The protobuf message for the type.""" + def protobuf_message(self) -> Message: + """The matching protobuf message for the Python type.""" return wrappers_pb2.Int64Value def to_protobuf(self, value: object) -> wrappers_pb2.Int64Value: @@ -116,8 +116,8 @@ def python_type(self) -> type: return str @property - def protobuf_message(self) -> type: - """The protobuf message for the type.""" + def protobuf_message(self) -> Message: + """The matching protobuf message for the Python type.""" return wrappers_pb2.StringValue def to_protobuf(self, value: object) -> wrappers_pb2.StringValue: From e9fe18ced1dcbfc35fc28da9de089cb4c7ccfb17 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Mon, 19 May 2025 12:09:17 -0500 Subject: [PATCH 28/39] Simple naive type-fix Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index ded2f1a..ab34cfc 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -19,7 +19,7 @@ def python_type(self) -> type: """The Python type that this converter handles.""" @property - def protobuf_message(self) -> Message: + def protobuf_message(self) -> type[Message]: """The matching protobuf message for the Python type.""" def to_protobuf(self, value: object) -> Message: @@ -40,7 +40,7 @@ def python_type(self) -> type: return bool @property - def protobuf_message(self) -> Message: + def protobuf_message(self) -> type[Message]: """The matching protobuf message for the Python type.""" return wrappers_pb2.BoolValue @@ -59,7 +59,7 @@ def python_type(self) -> type: return bytes @property - def protobuf_message(self) -> Message: + def protobuf_message(self) -> type[Message]: """The matching protobuf message for the Python type.""" return wrappers_pb2.BytesValue @@ -78,7 +78,7 @@ def python_type(self) -> type: return float @property - def protobuf_message(self) -> Message: + def protobuf_message(self) -> type[Message]: """The matching protobuf message for the Python type.""" return wrappers_pb2.DoubleValue @@ -97,7 +97,7 @@ def python_type(self) -> type: return int @property - def protobuf_message(self) -> Message: + def protobuf_message(self) -> type[Message]: """The matching protobuf message for the Python type.""" return wrappers_pb2.Int64Value @@ -116,7 +116,7 @@ def python_type(self) -> type: return str @property - def protobuf_message(self) -> Message: + def protobuf_message(self) -> type[Message]: """The matching protobuf message for the Python type.""" return wrappers_pb2.StringValue From dc0c5527df915deb164b9e7588d8d7425d23fcd2 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Tue, 20 May 2025 12:47:57 -0500 Subject: [PATCH 29/39] Refactor converters as an ABC family Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 146 ++++++++++++++++++++++++------------- 1 file changed, 95 insertions(+), 51 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index ab34cfc..2401b34 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -3,36 +3,69 @@ from __future__ import annotations import logging -from typing import Protocol +from abc import ABC, abstractmethod +from typing import Any, Generic, TypeVar from google.protobuf import any_pb2, wrappers_pb2 from google.protobuf.message import Message +TPythonType = TypeVar("TPythonType", covariant=True) +TProtobufType = TypeVar("TProtobufType", bound=Message, covariant=True) +TBuiltinProtobufType = TypeVar( + "TBuiltinProtobufType", + wrappers_pb2.BoolValue, + wrappers_pb2.BytesValue, + wrappers_pb2.DoubleValue, + wrappers_pb2.Int64Value, + wrappers_pb2.StringValue, +) + _logger = logging.getLogger(__name__) -class Converter(Protocol): +def _from_python_builtin(protobuf_message: TBuiltinProtobufType, python_value: TPythonType) -> TBuiltinProtobufType: + wrapped_value = protobuf_message(value=python_value) + as_any = any_pb2.Any() + as_any.Pack(wrapped_value) + return as_any + + +def _to_python_builtin(protobuf_value: any_pb2.Any, protobuf_message: TBuiltinProtobufType) -> TPythonType: + did_unpack = protobuf_value.Unpack(protobuf_message) + if not did_unpack: + raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") + return protobuf_message.value + + +class Converter(Generic[TPythonType, TProtobufType], ABC): """A class that defines how to convert between Python and protobuf types.""" @property - def python_type(self) -> type: + @abstractmethod + def python_type(self) -> TPythonType: """The Python type that this converter handles.""" @property - def protobuf_message(self) -> type[Message]: + @abstractmethod + def protobuf_message(self) -> TProtobufType: """The matching protobuf message for the Python type.""" - def to_protobuf(self, value: object) -> Message: - """Convert the Python object to its type-specific protobuf message.""" - @property def protobuf_typename(self) -> str: """The protobuf name for the type.""" - return self.protobuf_message.DESCRIPTOR.full_name + return self.protobuf_message.DESCRIPTOR.full_name # type: ignore[no-any-return] + + @abstractmethod + def to_protobuf(self, python_value: Any) -> TProtobufType: + """Convert the Python object to its type-specific protobuf message.""" + @abstractmethod + def to_python(self, protobuf_value: any_pb2.Any) -> TPythonType: + """Convert the protobuf message to its matching Python type.""" -class BoolConverter(Converter): - """A converter for bool types.""" + +class BoolConverter(Converter[bool, wrappers_pb2.BoolValue]): + """A converter for boolean types.""" @property def python_type(self) -> type: @@ -40,18 +73,22 @@ def python_type(self) -> type: return bool @property - def protobuf_message(self) -> type[Message]: + def protobuf_message(self) -> type: """The matching protobuf message for the Python type.""" return wrappers_pb2.BoolValue - def to_protobuf(self, value: object) -> wrappers_pb2.BoolValue: + def to_protobuf(self, python_value: bool) -> wrappers_pb2.BoolValue: """Convert a bool to a protobuf BoolValue.""" - assert isinstance(value, bool), f"Expected bool, got {type(value)}" - return self.protobuf_message(value=value) + return _from_python_builtin(self.protobuf_message, python_value) + + def to_python(self, protobuf_value: any_pb2.Any) -> bool: + """Convert the protobuf message to a Python bool.""" + protobuf_message = self.protobuf_message() + return _to_python_builtin(protobuf_value, protobuf_message) -class BytesConverter(Converter): - """A converter for bytes types.""" +class BytesConverter(Converter[bytes, wrappers_pb2.BytesValue]): + """A converter for byte string types.""" @property def python_type(self) -> type: @@ -59,18 +96,22 @@ def python_type(self) -> type: return bytes @property - def protobuf_message(self) -> type[Message]: + def protobuf_message(self) -> type: """The matching protobuf message for the Python type.""" return wrappers_pb2.BytesValue - def to_protobuf(self, value: object) -> wrappers_pb2.BytesValue: + def to_protobuf(self, python_value: bytes) -> wrappers_pb2.BytesValue: """Convert bytes to a protobuf BytesValue.""" - assert isinstance(value, bytes), f"Expected bytes, got {type(value)}" - return self.protobuf_message(value=value) + return _from_python_builtin(self.protobuf_message, python_value) + + def to_python(self, protobuf_value: any_pb2.Any) -> bytes: + """Convert the protobuf message to Python bytes.""" + protobuf_message = self.protobuf_message() + return _to_python_builtin(protobuf_value, protobuf_message) -class FloatConverter(Converter): - """A converter for float types.""" +class FloatConverter(Converter[float, wrappers_pb2.DoubleValue]): + """A converter for floating point types.""" @property def python_type(self) -> type: @@ -78,18 +119,22 @@ def python_type(self) -> type: return float @property - def protobuf_message(self) -> type[Message]: + def protobuf_message(self) -> type: """The matching protobuf message for the Python type.""" return wrappers_pb2.DoubleValue - def to_protobuf(self, value: object) -> wrappers_pb2.DoubleValue: + def to_protobuf(self, python_value: float) -> wrappers_pb2.DoubleValue: """Convert a float to a protobuf DoubleValue.""" - assert isinstance(value, float), f"Expected float, got {type(value)}" - return self.protobuf_message(value=value) + return _from_python_builtin(self.protobuf_message, python_value) + def to_python(self, protobuf_value: any_pb2.Any) -> float: + """Convert the protobuf message to a Python float.""" + protobuf_message = self.protobuf_message() + return _to_python_builtin(protobuf_value, protobuf_message) -class IntConverter(Converter): - """A converter for int types.""" + +class IntConverter(Converter[int, wrappers_pb2.Int64Value]): + """A converter for integer types.""" @property def python_type(self) -> type: @@ -97,18 +142,22 @@ def python_type(self) -> type: return int @property - def protobuf_message(self) -> type[Message]: + def protobuf_message(self) -> type: """The matching protobuf message for the Python type.""" return wrappers_pb2.Int64Value - def to_protobuf(self, value: object) -> wrappers_pb2.Int64Value: + def to_protobuf(self, python_value: int) -> wrappers_pb2.Int64Value: """Convert an int to a protobuf Int64Value.""" - assert isinstance(value, int), f"Expected int, got {type(value)}" - return self.protobuf_message(value=value) + return _from_python_builtin(self.protobuf_message, python_value) + + def to_python(self, protobuf_value: any_pb2.Any) -> int: + """Convert the protobuf message to a Python int.""" + protobuf_message = self.protobuf_message() + return _to_python_builtin(protobuf_value, protobuf_message) -class StrConverter(Converter): - """A converter for str types.""" +class StrConverter(Converter[str, wrappers_pb2.StringValue]): + """A converter for text string types.""" @property def python_type(self) -> type: @@ -116,17 +165,21 @@ def python_type(self) -> type: return str @property - def protobuf_message(self) -> type[Message]: + def protobuf_message(self) -> type: """The matching protobuf message for the Python type.""" return wrappers_pb2.StringValue - def to_protobuf(self, value: object) -> wrappers_pb2.StringValue: + def to_protobuf(self, python_value: str) -> wrappers_pb2.StringValue: """Convert a str to a protobuf StringValue.""" - assert isinstance(value, str), f"Expected str, got {type(value)}" - return self.protobuf_message(value=value) + return _from_python_builtin(self.protobuf_message, python_value) + + def to_python(self, protobuf_value: any_pb2.Any) -> str: + """Convert the protobuf message to a Python string.""" + protobuf_message = self.protobuf_message() + return _to_python_builtin(protobuf_value, protobuf_message) -_CONVERTIBLE_TYPES = [ +_CONVERTIBLE_TYPES: list[Converter] = [ BoolConverter(), BytesConverter(), FloatConverter(), @@ -134,10 +187,9 @@ def to_protobuf(self, value: object) -> wrappers_pb2.StringValue: StrConverter(), ] +# FFV -- consider adding a RegisterConverter mechanism _CONVERTER_FOR_PYTHON_TYPE = {entry.python_type: entry for entry in _CONVERTIBLE_TYPES} - _CONVERTER_FOR_GRPC_TYPE = {entry.protobuf_typename: entry for entry in _CONVERTIBLE_TYPES} - _SUPPORTED_PYTHON_TYPES = _CONVERTER_FOR_PYTHON_TYPE.keys() @@ -154,11 +206,8 @@ def to_any(python_value: object) -> any_pb2.Any: ) _logger.debug(f"Best matching type for '{repr(python_value)}' resolved to {best_matching_type}") - the_any = any_pb2.Any() converter = _CONVERTER_FOR_PYTHON_TYPE[best_matching_type] - wrapped_value = converter.to_protobuf(python_value) - the_any.Pack(wrapped_value) - return the_any + return converter.to_protobuf(python_value) def from_any(protobuf_any: any_pb2.Any) -> object: @@ -170,9 +219,4 @@ def from_any(protobuf_any: any_pb2.Any) -> object: _logger.debug(f"Unpacking type '{underlying_typename}'") converter = _CONVERTER_FOR_GRPC_TYPE[underlying_typename] - protobuf_message = converter.protobuf_message() - did_unpack = protobuf_any.Unpack(protobuf_message) - if not did_unpack: - raise ValueError(f"Failed to unpack Any with underlying type '{underlying_typename}'") - - return protobuf_message.value + return converter.to_python(protobuf_any) From 4f3a208369fe89c9be24e9fb1e334daefb8577c9 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Tue, 20 May 2025 13:54:18 -0500 Subject: [PATCH 30/39] Converting builtins with a shared function, still mypy errors Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 67 ++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 2401b34..1b91f41 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -4,33 +4,50 @@ import logging from abc import ABC, abstractmethod -from typing import Any, Generic, TypeVar +from typing import Any, Generic, Type, TypeVar from google.protobuf import any_pb2, wrappers_pb2 from google.protobuf.message import Message TPythonType = TypeVar("TPythonType", covariant=True) TProtobufType = TypeVar("TProtobufType", bound=Message, covariant=True) -TBuiltinProtobufType = TypeVar( - "TBuiltinProtobufType", + +TBuiltinPythonType = TypeVar( + "TBuiltinPythonType", + bool, + bytes, + float, + int, + str, +) +TBuiltinProtobufMessage = TypeVar( + "TBuiltinProtobufMessage", wrappers_pb2.BoolValue, wrappers_pb2.BytesValue, wrappers_pb2.DoubleValue, wrappers_pb2.Int64Value, wrappers_pb2.StringValue, ) +TBuiltinProtobufType = TypeVar( + "TBuiltinProtobufType", + Type[wrappers_pb2.BoolValue], + Type[wrappers_pb2.BytesValue], + Type[wrappers_pb2.DoubleValue], + Type[wrappers_pb2.Int64Value], + Type[wrappers_pb2.StringValue], +) _logger = logging.getLogger(__name__) -def _from_python_builtin(protobuf_message: TBuiltinProtobufType, python_value: TPythonType) -> TBuiltinProtobufType: +def _from_python_builtin(protobuf_message: TBuiltinProtobufType, python_value: TBuiltinPythonType) -> any_pb2.Any: wrapped_value = protobuf_message(value=python_value) as_any = any_pb2.Any() as_any.Pack(wrapped_value) return as_any -def _to_python_builtin(protobuf_value: any_pb2.Any, protobuf_message: TBuiltinProtobufType) -> TPythonType: +def _to_python_builtin(protobuf_value: any_pb2.Any, protobuf_message: TBuiltinProtobufMessage) -> TBuiltinPythonType: did_unpack = protobuf_value.Unpack(protobuf_message) if not did_unpack: raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") @@ -42,12 +59,12 @@ class Converter(Generic[TPythonType, TProtobufType], ABC): @property @abstractmethod - def python_type(self) -> TPythonType: + def python_type(self) -> Type[TPythonType]: """The Python type that this converter handles.""" @property @abstractmethod - def protobuf_message(self) -> TProtobufType: + def protobuf_message(self) -> Type[TProtobufType]: """The matching protobuf message for the Python type.""" @property @@ -56,8 +73,8 @@ def protobuf_typename(self) -> str: return self.protobuf_message.DESCRIPTOR.full_name # type: ignore[no-any-return] @abstractmethod - def to_protobuf(self, python_value: Any) -> TProtobufType: - """Convert the Python object to its type-specific protobuf message.""" + def to_protobuf(self, python_value: Any) -> any_pb2.Any: + """Convert the Python object to its type-specific protobuf message and pack it into an any_pb2.Any.""" @abstractmethod def to_python(self, protobuf_value: any_pb2.Any) -> TPythonType: @@ -68,16 +85,16 @@ class BoolConverter(Converter[bool, wrappers_pb2.BoolValue]): """A converter for boolean types.""" @property - def python_type(self) -> type: + def python_type(self) -> Type[bool]: """The Python type that this converter handles.""" return bool @property - def protobuf_message(self) -> type: + def protobuf_message(self) -> Type[wrappers_pb2.BoolValue]: """The matching protobuf message for the Python type.""" return wrappers_pb2.BoolValue - def to_protobuf(self, python_value: bool) -> wrappers_pb2.BoolValue: + def to_protobuf(self, python_value: bool) -> any_pb2.Any: """Convert a bool to a protobuf BoolValue.""" return _from_python_builtin(self.protobuf_message, python_value) @@ -91,16 +108,16 @@ class BytesConverter(Converter[bytes, wrappers_pb2.BytesValue]): """A converter for byte string types.""" @property - def python_type(self) -> type: + def python_type(self) -> Type[bytes]: """The Python type that this converter handles.""" return bytes @property - def protobuf_message(self) -> type: + def protobuf_message(self) -> Type[wrappers_pb2.BytesValue]: """The matching protobuf message for the Python type.""" return wrappers_pb2.BytesValue - def to_protobuf(self, python_value: bytes) -> wrappers_pb2.BytesValue: + def to_protobuf(self, python_value: bytes) -> any_pb2.Any: """Convert bytes to a protobuf BytesValue.""" return _from_python_builtin(self.protobuf_message, python_value) @@ -114,16 +131,16 @@ class FloatConverter(Converter[float, wrappers_pb2.DoubleValue]): """A converter for floating point types.""" @property - def python_type(self) -> type: + def python_type(self) -> Type[float]: """The Python type that this converter handles.""" return float @property - def protobuf_message(self) -> type: + def protobuf_message(self) -> Type[wrappers_pb2.DoubleValue]: """The matching protobuf message for the Python type.""" return wrappers_pb2.DoubleValue - def to_protobuf(self, python_value: float) -> wrappers_pb2.DoubleValue: + def to_protobuf(self, python_value: float) -> any_pb2.Any: """Convert a float to a protobuf DoubleValue.""" return _from_python_builtin(self.protobuf_message, python_value) @@ -137,16 +154,16 @@ class IntConverter(Converter[int, wrappers_pb2.Int64Value]): """A converter for integer types.""" @property - def python_type(self) -> type: + def python_type(self) -> Type[int]: """The Python type that this converter handles.""" return int @property - def protobuf_message(self) -> type: + def protobuf_message(self) -> Type[wrappers_pb2.Int64Value]: """The matching protobuf message for the Python type.""" return wrappers_pb2.Int64Value - def to_protobuf(self, python_value: int) -> wrappers_pb2.Int64Value: + def to_protobuf(self, python_value: int) -> any_pb2.Any: """Convert an int to a protobuf Int64Value.""" return _from_python_builtin(self.protobuf_message, python_value) @@ -160,16 +177,16 @@ class StrConverter(Converter[str, wrappers_pb2.StringValue]): """A converter for text string types.""" @property - def python_type(self) -> type: + def python_type(self) -> Type[str]: """The Python type that this converter handles.""" return str @property - def protobuf_message(self) -> type: + def protobuf_message(self) -> Type[wrappers_pb2.StringValue]: """The matching protobuf message for the Python type.""" return wrappers_pb2.StringValue - def to_protobuf(self, python_value: str) -> wrappers_pb2.StringValue: + def to_protobuf(self, python_value: str) -> any_pb2.Any: """Convert a str to a protobuf StringValue.""" return _from_python_builtin(self.protobuf_message, python_value) @@ -179,7 +196,7 @@ def to_python(self, protobuf_value: any_pb2.Any) -> str: return _to_python_builtin(protobuf_value, protobuf_message) -_CONVERTIBLE_TYPES: list[Converter] = [ +_CONVERTIBLE_TYPES = [ BoolConverter(), BytesConverter(), FloatConverter(), From 91151005811e646fcfbb048a5718ec1e1f3bd7b4 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Tue, 20 May 2025 14:12:19 -0500 Subject: [PATCH 31/39] Do not share code between the builtin converters because type hinting is too difficult (for me) Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 119 +++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 64 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 1b91f41..2daef1b 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -12,50 +12,11 @@ TPythonType = TypeVar("TPythonType", covariant=True) TProtobufType = TypeVar("TProtobufType", bound=Message, covariant=True) -TBuiltinPythonType = TypeVar( - "TBuiltinPythonType", - bool, - bytes, - float, - int, - str, -) -TBuiltinProtobufMessage = TypeVar( - "TBuiltinProtobufMessage", - wrappers_pb2.BoolValue, - wrappers_pb2.BytesValue, - wrappers_pb2.DoubleValue, - wrappers_pb2.Int64Value, - wrappers_pb2.StringValue, -) -TBuiltinProtobufType = TypeVar( - "TBuiltinProtobufType", - Type[wrappers_pb2.BoolValue], - Type[wrappers_pb2.BytesValue], - Type[wrappers_pb2.DoubleValue], - Type[wrappers_pb2.Int64Value], - Type[wrappers_pb2.StringValue], -) - _logger = logging.getLogger(__name__) -def _from_python_builtin(protobuf_message: TBuiltinProtobufType, python_value: TBuiltinPythonType) -> any_pb2.Any: - wrapped_value = protobuf_message(value=python_value) - as_any = any_pb2.Any() - as_any.Pack(wrapped_value) - return as_any - - -def _to_python_builtin(protobuf_value: any_pb2.Any, protobuf_message: TBuiltinProtobufMessage) -> TBuiltinPythonType: - did_unpack = protobuf_value.Unpack(protobuf_message) - if not did_unpack: - raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") - return protobuf_message.value - - class Converter(Generic[TPythonType, TProtobufType], ABC): - """A class that defines how to convert between Python and protobuf types.""" + """A class that defines how to convert between Python objects and protobuf Any messages.""" @property @abstractmethod @@ -65,7 +26,7 @@ def python_type(self) -> Type[TPythonType]: @property @abstractmethod def protobuf_message(self) -> Type[TProtobufType]: - """The matching protobuf message for the Python type.""" + """The type-specific protobuf message for the Python type.""" @property def protobuf_typename(self) -> str: @@ -91,17 +52,23 @@ def python_type(self) -> Type[bool]: @property def protobuf_message(self) -> Type[wrappers_pb2.BoolValue]: - """The matching protobuf message for the Python type.""" + """The type-specific protobuf message for the Python type.""" return wrappers_pb2.BoolValue def to_protobuf(self, python_value: bool) -> any_pb2.Any: - """Convert a bool to a protobuf BoolValue.""" - return _from_python_builtin(self.protobuf_message, python_value) + """Convert a bool to a protobuf Any.""" + wrapped_value = self.protobuf_message(value=python_value) + as_any = any_pb2.Any() + as_any.Pack(wrapped_value) + return as_any def to_python(self, protobuf_value: any_pb2.Any) -> bool: """Convert the protobuf message to a Python bool.""" protobuf_message = self.protobuf_message() - return _to_python_builtin(protobuf_value, protobuf_message) + did_unpack = protobuf_value.Unpack(protobuf_message) + if not did_unpack: + raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") + return protobuf_message.value class BytesConverter(Converter[bytes, wrappers_pb2.BytesValue]): @@ -114,17 +81,23 @@ def python_type(self) -> Type[bytes]: @property def protobuf_message(self) -> Type[wrappers_pb2.BytesValue]: - """The matching protobuf message for the Python type.""" + """The type-specific protobuf message for the Python type.""" return wrappers_pb2.BytesValue def to_protobuf(self, python_value: bytes) -> any_pb2.Any: - """Convert bytes to a protobuf BytesValue.""" - return _from_python_builtin(self.protobuf_message, python_value) + """Convert bytes to a protobuf Any.""" + wrapped_value = self.protobuf_message(value=python_value) + as_any = any_pb2.Any() + as_any.Pack(wrapped_value) + return as_any def to_python(self, protobuf_value: any_pb2.Any) -> bytes: """Convert the protobuf message to Python bytes.""" protobuf_message = self.protobuf_message() - return _to_python_builtin(protobuf_value, protobuf_message) + did_unpack = protobuf_value.Unpack(protobuf_message) + if not did_unpack: + raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") + return protobuf_message.value class FloatConverter(Converter[float, wrappers_pb2.DoubleValue]): @@ -137,17 +110,23 @@ def python_type(self) -> Type[float]: @property def protobuf_message(self) -> Type[wrappers_pb2.DoubleValue]: - """The matching protobuf message for the Python type.""" + """The type-specific protobuf message for the Python type.""" return wrappers_pb2.DoubleValue def to_protobuf(self, python_value: float) -> any_pb2.Any: - """Convert a float to a protobuf DoubleValue.""" - return _from_python_builtin(self.protobuf_message, python_value) + """Convert a float to a protobuf Any.""" + wrapped_value = self.protobuf_message(value=python_value) + as_any = any_pb2.Any() + as_any.Pack(wrapped_value) + return as_any def to_python(self, protobuf_value: any_pb2.Any) -> float: """Convert the protobuf message to a Python float.""" protobuf_message = self.protobuf_message() - return _to_python_builtin(protobuf_value, protobuf_message) + did_unpack = protobuf_value.Unpack(protobuf_message) + if not did_unpack: + raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") + return protobuf_message.value class IntConverter(Converter[int, wrappers_pb2.Int64Value]): @@ -160,17 +139,23 @@ def python_type(self) -> Type[int]: @property def protobuf_message(self) -> Type[wrappers_pb2.Int64Value]: - """The matching protobuf message for the Python type.""" + """The type-specific protobuf message for the Python type.""" return wrappers_pb2.Int64Value def to_protobuf(self, python_value: int) -> any_pb2.Any: - """Convert an int to a protobuf Int64Value.""" - return _from_python_builtin(self.protobuf_message, python_value) + """Convert an int to a protobuf Any.""" + wrapped_value = self.protobuf_message(value=python_value) + as_any = any_pb2.Any() + as_any.Pack(wrapped_value) + return as_any def to_python(self, protobuf_value: any_pb2.Any) -> int: """Convert the protobuf message to a Python int.""" protobuf_message = self.protobuf_message() - return _to_python_builtin(protobuf_value, protobuf_message) + did_unpack = protobuf_value.Unpack(protobuf_message) + if not did_unpack: + raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") + return protobuf_message.value class StrConverter(Converter[str, wrappers_pb2.StringValue]): @@ -183,19 +168,26 @@ def python_type(self) -> Type[str]: @property def protobuf_message(self) -> Type[wrappers_pb2.StringValue]: - """The matching protobuf message for the Python type.""" + """The type-specific protobuf message for the Python type.""" return wrappers_pb2.StringValue def to_protobuf(self, python_value: str) -> any_pb2.Any: - """Convert a str to a protobuf StringValue.""" - return _from_python_builtin(self.protobuf_message, python_value) + """Convert a str to a protobuf Any.""" + wrapped_value = self.protobuf_message(value=python_value) + as_any = any_pb2.Any() + as_any.Pack(wrapped_value) + return as_any def to_python(self, protobuf_value: any_pb2.Any) -> str: """Convert the protobuf message to a Python string.""" protobuf_message = self.protobuf_message() - return _to_python_builtin(protobuf_value, protobuf_message) + did_unpack = protobuf_value.Unpack(protobuf_message) + if not did_unpack: + raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") + return protobuf_message.value +# FFV -- consider adding a RegisterConverter mechanism _CONVERTIBLE_TYPES = [ BoolConverter(), BytesConverter(), @@ -204,13 +196,12 @@ def to_python(self, protobuf_value: any_pb2.Any) -> str: StrConverter(), ] -# FFV -- consider adding a RegisterConverter mechanism _CONVERTER_FOR_PYTHON_TYPE = {entry.python_type: entry for entry in _CONVERTIBLE_TYPES} _CONVERTER_FOR_GRPC_TYPE = {entry.protobuf_typename: entry for entry in _CONVERTIBLE_TYPES} _SUPPORTED_PYTHON_TYPES = _CONVERTER_FOR_PYTHON_TYPE.keys() -def to_any(python_value: object) -> any_pb2.Any: +def to_any(python_value: Any) -> any_pb2.Any: """Convert a Python object to a protobuf Any.""" underlying_parents = type(python_value).mro() # This covers enum.IntEnum and similar @@ -227,7 +218,7 @@ def to_any(python_value: object) -> any_pb2.Any: return converter.to_protobuf(python_value) -def from_any(protobuf_any: any_pb2.Any) -> object: +def from_any(protobuf_any: any_pb2.Any) -> Any: """Convert a protobuf Any to a Python object.""" if not isinstance(protobuf_any, any_pb2.Any): raise ValueError(f"Unexpected type: {type(protobuf_any)}") From 6d8318bcce162714d65eef78d134f26ce980941d Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Tue, 20 May 2025 14:17:22 -0500 Subject: [PATCH 32/39] Address the linters Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 2daef1b..7f904c9 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -9,23 +9,23 @@ from google.protobuf import any_pb2, wrappers_pb2 from google.protobuf.message import Message -TPythonType = TypeVar("TPythonType", covariant=True) -TProtobufType = TypeVar("TProtobufType", bound=Message, covariant=True) +TPythonType_co = TypeVar("TPythonType_co", covariant=True) +TProtobufType_co = TypeVar("TProtobufType_co", bound=Message, covariant=True) _logger = logging.getLogger(__name__) -class Converter(Generic[TPythonType, TProtobufType], ABC): +class Converter(Generic[TPythonType_co, TProtobufType_co], ABC): """A class that defines how to convert between Python objects and protobuf Any messages.""" @property @abstractmethod - def python_type(self) -> Type[TPythonType]: + def python_type(self) -> Type[TPythonType_co]: """The Python type that this converter handles.""" @property @abstractmethod - def protobuf_message(self) -> Type[TProtobufType]: + def protobuf_message(self) -> Type[TProtobufType_co]: """The type-specific protobuf message for the Python type.""" @property @@ -35,10 +35,10 @@ def protobuf_typename(self) -> str: @abstractmethod def to_protobuf(self, python_value: Any) -> any_pb2.Any: - """Convert the Python object to its type-specific protobuf message and pack it into an any_pb2.Any.""" + """Convert the Python object to its type-specific message and pack it as any_pb2.Any.""" @abstractmethod - def to_python(self, protobuf_value: any_pb2.Any) -> TPythonType: + def to_python(self, protobuf_value: any_pb2.Any) -> TPythonType_co: """Convert the protobuf message to its matching Python type.""" From 33136917c3c6d4e066e43dcb194531c28d885821 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Tue, 20 May 2025 14:29:29 -0500 Subject: [PATCH 33/39] Follow naming convention for covariant typevars Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 7f904c9..75bd760 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -9,23 +9,23 @@ from google.protobuf import any_pb2, wrappers_pb2 from google.protobuf.message import Message -TPythonType_co = TypeVar("TPythonType_co", covariant=True) -TProtobufType_co = TypeVar("TProtobufType_co", bound=Message, covariant=True) +_TPythonType_co = TypeVar("_TPythonType_co", covariant=True) +_TProtobufType_co = TypeVar("_TProtobufType_co", bound=Message, covariant=True) _logger = logging.getLogger(__name__) -class Converter(Generic[TPythonType_co, TProtobufType_co], ABC): +class Converter(Generic[_TPythonType_co, _TProtobufType_co], ABC): """A class that defines how to convert between Python objects and protobuf Any messages.""" @property @abstractmethod - def python_type(self) -> Type[TPythonType_co]: + def python_type(self) -> Type[_TPythonType_co]: """The Python type that this converter handles.""" @property @abstractmethod - def protobuf_message(self) -> Type[TProtobufType_co]: + def protobuf_message(self) -> Type[_TProtobufType_co]: """The type-specific protobuf message for the Python type.""" @property @@ -38,7 +38,7 @@ def to_protobuf(self, python_value: Any) -> any_pb2.Any: """Convert the Python object to its type-specific message and pack it as any_pb2.Any.""" @abstractmethod - def to_python(self, protobuf_value: any_pb2.Any) -> TPythonType_co: + def to_python(self, protobuf_value: any_pb2.Any) -> _TPythonType_co: """Convert the protobuf message to its matching Python type.""" From c1324ae178e6f99549328e4f5fbecbbd0238c3c0 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Tue, 20 May 2025 14:35:50 -0500 Subject: [PATCH 34/39] Rely on set_value throwing for test___unopened_panel___set_value___sets_value Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- tests/unit/test_streamlit_panel.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test_streamlit_panel.py b/tests/unit/test_streamlit_panel.py index d85de8f..0f2de89 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -51,9 +51,8 @@ def test___unopened_panel___set_value___sets_value( value_id = "test_id" string_value = "test_value" - panel.set_value(value_id, string_value) - assert panel.get_value(value_id) == string_value + panel.set_value(value_id, string_value) def test___unopened_panel___get_unset_value___raises_exception( From d5d4c51c80e5b94d4004ac90644385f5c0e59add Mon Sep 17 00:00:00 2001 From: Brad Keryan Date: Tue, 20 May 2025 17:29:51 -0500 Subject: [PATCH 35/39] converters: Refactor message->Any conversion --- src/nipanel/_converters.py | 46 ++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 75bd760..eac0628 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -33,8 +33,15 @@ def protobuf_typename(self) -> str: """The protobuf name for the type.""" return self.protobuf_message.DESCRIPTOR.full_name # type: ignore[no-any-return] + def to_protobuf_any(self, python_value: Any) -> any_pb2.Any: + """Convert the Python object to its type-specific message and pack it as any_pb2.Any.""" + message = self.to_protobuf_message(python_value) + as_any = any_pb2.Any() + as_any.Pack(message) + return as_any + @abstractmethod - def to_protobuf(self, python_value: Any) -> any_pb2.Any: + def to_protobuf_message(self, python_value: Any) -> Message: """Convert the Python object to its type-specific message and pack it as any_pb2.Any.""" @abstractmethod @@ -55,12 +62,9 @@ def protobuf_message(self) -> Type[wrappers_pb2.BoolValue]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.BoolValue - def to_protobuf(self, python_value: bool) -> any_pb2.Any: + def to_protobuf_message(self, python_value: bool) -> Message: """Convert a bool to a protobuf Any.""" - wrapped_value = self.protobuf_message(value=python_value) - as_any = any_pb2.Any() - as_any.Pack(wrapped_value) - return as_any + return self.protobuf_message(value=python_value) def to_python(self, protobuf_value: any_pb2.Any) -> bool: """Convert the protobuf message to a Python bool.""" @@ -84,12 +88,9 @@ def protobuf_message(self) -> Type[wrappers_pb2.BytesValue]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.BytesValue - def to_protobuf(self, python_value: bytes) -> any_pb2.Any: + def to_protobuf_message(self, python_value: bytes) -> Message: """Convert bytes to a protobuf Any.""" - wrapped_value = self.protobuf_message(value=python_value) - as_any = any_pb2.Any() - as_any.Pack(wrapped_value) - return as_any + return self.protobuf_message(value=python_value) def to_python(self, protobuf_value: any_pb2.Any) -> bytes: """Convert the protobuf message to Python bytes.""" @@ -113,12 +114,9 @@ def protobuf_message(self) -> Type[wrappers_pb2.DoubleValue]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.DoubleValue - def to_protobuf(self, python_value: float) -> any_pb2.Any: + def to_protobuf_message(self, python_value: float) -> Message: """Convert a float to a protobuf Any.""" - wrapped_value = self.protobuf_message(value=python_value) - as_any = any_pb2.Any() - as_any.Pack(wrapped_value) - return as_any + return self.protobuf_message(value=python_value) def to_python(self, protobuf_value: any_pb2.Any) -> float: """Convert the protobuf message to a Python float.""" @@ -142,12 +140,9 @@ def protobuf_message(self) -> Type[wrappers_pb2.Int64Value]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.Int64Value - def to_protobuf(self, python_value: int) -> any_pb2.Any: + def to_protobuf_message(self, python_value: int) -> Message: """Convert an int to a protobuf Any.""" - wrapped_value = self.protobuf_message(value=python_value) - as_any = any_pb2.Any() - as_any.Pack(wrapped_value) - return as_any + return self.protobuf_message(value=python_value) def to_python(self, protobuf_value: any_pb2.Any) -> int: """Convert the protobuf message to a Python int.""" @@ -171,12 +166,9 @@ def protobuf_message(self) -> Type[wrappers_pb2.StringValue]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.StringValue - def to_protobuf(self, python_value: str) -> any_pb2.Any: + def to_protobuf_message(self, python_value: str) -> Message: """Convert a str to a protobuf Any.""" - wrapped_value = self.protobuf_message(value=python_value) - as_any = any_pb2.Any() - as_any.Pack(wrapped_value) - return as_any + return self.protobuf_message(value=python_value) def to_python(self, protobuf_value: any_pb2.Any) -> str: """Convert the protobuf message to a Python string.""" @@ -215,7 +207,7 @@ def to_any(python_value: Any) -> any_pb2.Any: _logger.debug(f"Best matching type for '{repr(python_value)}' resolved to {best_matching_type}") converter = _CONVERTER_FOR_PYTHON_TYPE[best_matching_type] - return converter.to_protobuf(python_value) + return converter.to_protobuf_any(python_value) def from_any(protobuf_any: any_pb2.Any) -> Any: From ae54c82d24ca66e6bc2b08406956dab9db9f842c Mon Sep 17 00:00:00 2001 From: Brad Keryan Date: Tue, 20 May 2025 17:40:52 -0500 Subject: [PATCH 36/39] converters: Refactor to_python any handling --- src/nipanel/_converters.py | 64 ++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index eac0628..eb5df13 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -9,23 +9,23 @@ from google.protobuf import any_pb2, wrappers_pb2 from google.protobuf.message import Message -_TPythonType_co = TypeVar("_TPythonType_co", covariant=True) -_TProtobufType_co = TypeVar("_TProtobufType_co", bound=Message, covariant=True) +_TPythonType = TypeVar("_TPythonType") +_TProtobufType = TypeVar("_TProtobufType", bound=Message) _logger = logging.getLogger(__name__) -class Converter(Generic[_TPythonType_co, _TProtobufType_co], ABC): +class Converter(Generic[_TPythonType, _TProtobufType], ABC): """A class that defines how to convert between Python objects and protobuf Any messages.""" @property @abstractmethod - def python_type(self) -> Type[_TPythonType_co]: + def python_type(self) -> Type[_TPythonType]: """The Python type that this converter handles.""" @property @abstractmethod - def protobuf_message(self) -> Type[_TProtobufType_co]: + def protobuf_message(self) -> Type[_TProtobufType]: """The type-specific protobuf message for the Python type.""" @property @@ -44,9 +44,17 @@ def to_protobuf_any(self, python_value: Any) -> any_pb2.Any: def to_protobuf_message(self, python_value: Any) -> Message: """Convert the Python object to its type-specific message and pack it as any_pb2.Any.""" + def to_python(self, protobuf_value: any_pb2.Any) -> _TPythonType: + """Convert the protobuf Any message to its matching Python type.""" + protobuf_message = self.protobuf_message() + did_unpack = protobuf_value.Unpack(protobuf_message) + if not did_unpack: + raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") + return self.to_python_value(protobuf_message) + @abstractmethod - def to_python(self, protobuf_value: any_pb2.Any) -> _TPythonType_co: - """Convert the protobuf message to its matching Python type.""" + def to_python_value(self, protobuf_message: _TProtobufType) -> _TPythonType: + """Convert the protobuf wrapper message to its matching Python type.""" class BoolConverter(Converter[bool, wrappers_pb2.BoolValue]): @@ -66,13 +74,9 @@ def to_protobuf_message(self, python_value: bool) -> Message: """Convert a bool to a protobuf Any.""" return self.protobuf_message(value=python_value) - def to_python(self, protobuf_value: any_pb2.Any) -> bool: + def to_python_value(self, protobuf_value: wrappers_pb2.BoolValue) -> bool: """Convert the protobuf message to a Python bool.""" - protobuf_message = self.protobuf_message() - did_unpack = protobuf_value.Unpack(protobuf_message) - if not did_unpack: - raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") - return protobuf_message.value + return protobuf_value.value class BytesConverter(Converter[bytes, wrappers_pb2.BytesValue]): @@ -92,13 +96,9 @@ def to_protobuf_message(self, python_value: bytes) -> Message: """Convert bytes to a protobuf Any.""" return self.protobuf_message(value=python_value) - def to_python(self, protobuf_value: any_pb2.Any) -> bytes: + def to_python_value(self, protobuf_value: wrappers_pb2.BytesValue) -> bytes: """Convert the protobuf message to Python bytes.""" - protobuf_message = self.protobuf_message() - did_unpack = protobuf_value.Unpack(protobuf_message) - if not did_unpack: - raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") - return protobuf_message.value + return protobuf_value.value class FloatConverter(Converter[float, wrappers_pb2.DoubleValue]): @@ -118,13 +118,9 @@ def to_protobuf_message(self, python_value: float) -> Message: """Convert a float to a protobuf Any.""" return self.protobuf_message(value=python_value) - def to_python(self, protobuf_value: any_pb2.Any) -> float: + def to_python_value(self, protobuf_value: wrappers_pb2.DoubleValue) -> float: """Convert the protobuf message to a Python float.""" - protobuf_message = self.protobuf_message() - did_unpack = protobuf_value.Unpack(protobuf_message) - if not did_unpack: - raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") - return protobuf_message.value + return protobuf_value.value class IntConverter(Converter[int, wrappers_pb2.Int64Value]): @@ -144,13 +140,9 @@ def to_protobuf_message(self, python_value: int) -> Message: """Convert an int to a protobuf Any.""" return self.protobuf_message(value=python_value) - def to_python(self, protobuf_value: any_pb2.Any) -> int: + def to_python_value(self, protobuf_value: wrappers_pb2.Int64Value) -> int: """Convert the protobuf message to a Python int.""" - protobuf_message = self.protobuf_message() - did_unpack = protobuf_value.Unpack(protobuf_message) - if not did_unpack: - raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") - return protobuf_message.value + return protobuf_value.value class StrConverter(Converter[str, wrappers_pb2.StringValue]): @@ -170,17 +162,13 @@ def to_protobuf_message(self, python_value: str) -> Message: """Convert a str to a protobuf Any.""" return self.protobuf_message(value=python_value) - def to_python(self, protobuf_value: any_pb2.Any) -> str: + def to_python_value(self, protobuf_value: wrappers_pb2.StringValue) -> str: """Convert the protobuf message to a Python string.""" - protobuf_message = self.protobuf_message() - did_unpack = protobuf_value.Unpack(protobuf_message) - if not did_unpack: - raise ValueError(f"Failed to unpack Any with type '{protobuf_value.TypeName()}'") - return protobuf_message.value + return protobuf_value.value # FFV -- consider adding a RegisterConverter mechanism -_CONVERTIBLE_TYPES = [ +_CONVERTIBLE_TYPES: list[Converter[Any, Any]] = [ BoolConverter(), BytesConverter(), FloatConverter(), From 6ff26ea30a314bc3cd2df7e7d672a49e2d783fa9 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Wed, 21 May 2025 10:29:00 -0500 Subject: [PATCH 37/39] Clarify some docstrings Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index eb5df13..5d0f01d 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -42,7 +42,7 @@ def to_protobuf_any(self, python_value: Any) -> any_pb2.Any: @abstractmethod def to_protobuf_message(self, python_value: Any) -> Message: - """Convert the Python object to its type-specific message and pack it as any_pb2.Any.""" + """Convert the Python object to its type-specific message.""" def to_python(self, protobuf_value: any_pb2.Any) -> _TPythonType: """Convert the protobuf Any message to its matching Python type.""" @@ -71,7 +71,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.BoolValue]: return wrappers_pb2.BoolValue def to_protobuf_message(self, python_value: bool) -> Message: - """Convert a bool to a protobuf Any.""" + """Convert the Python bool to a protobuf wrappers_pb2.BoolValue.""" return self.protobuf_message(value=python_value) def to_python_value(self, protobuf_value: wrappers_pb2.BoolValue) -> bool: @@ -93,11 +93,11 @@ def protobuf_message(self) -> Type[wrappers_pb2.BytesValue]: return wrappers_pb2.BytesValue def to_protobuf_message(self, python_value: bytes) -> Message: - """Convert bytes to a protobuf Any.""" + """Convert the Python bytes string to a protobuf wrappers_pb2.BytesValue.""" return self.protobuf_message(value=python_value) def to_python_value(self, protobuf_value: wrappers_pb2.BytesValue) -> bytes: - """Convert the protobuf message to Python bytes.""" + """Convert the protobuf message to a Python bytes string.""" return protobuf_value.value @@ -115,7 +115,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.DoubleValue]: return wrappers_pb2.DoubleValue def to_protobuf_message(self, python_value: float) -> Message: - """Convert a float to a protobuf Any.""" + """Convert the Python float to a protobuf wrappers_pb2.DoubleValue.""" return self.protobuf_message(value=python_value) def to_python_value(self, protobuf_value: wrappers_pb2.DoubleValue) -> float: @@ -137,7 +137,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.Int64Value]: return wrappers_pb2.Int64Value def to_protobuf_message(self, python_value: int) -> Message: - """Convert an int to a protobuf Any.""" + """Convert the Python int to a protobuf wrappers_pb2.Int64Value.""" return self.protobuf_message(value=python_value) def to_python_value(self, protobuf_value: wrappers_pb2.Int64Value) -> int: @@ -159,7 +159,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.StringValue]: return wrappers_pb2.StringValue def to_protobuf_message(self, python_value: str) -> Message: - """Convert a str to a protobuf Any.""" + """Convert the Python str to a protobuf wrappers_pb2.StringValue.""" return self.protobuf_message(value=python_value) def to_python_value(self, protobuf_value: wrappers_pb2.StringValue) -> str: From 072a3ac889d0c622335bc5ce5bf60b0ee19d9f17 Mon Sep 17 00:00:00 2001 From: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> Date: Wed, 21 May 2025 10:33:05 -0500 Subject: [PATCH 38/39] Use object in the to_any() and from_any() signatures Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com> --- src/nipanel/_converters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 5d0f01d..705368e 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -181,7 +181,7 @@ def to_python_value(self, protobuf_value: wrappers_pb2.StringValue) -> str: _SUPPORTED_PYTHON_TYPES = _CONVERTER_FOR_PYTHON_TYPE.keys() -def to_any(python_value: Any) -> any_pb2.Any: +def to_any(python_value: object) -> any_pb2.Any: """Convert a Python object to a protobuf Any.""" underlying_parents = type(python_value).mro() # This covers enum.IntEnum and similar @@ -198,7 +198,7 @@ def to_any(python_value: Any) -> any_pb2.Any: return converter.to_protobuf_any(python_value) -def from_any(protobuf_any: any_pb2.Any) -> Any: +def from_any(protobuf_any: any_pb2.Any) -> object: """Convert a protobuf Any to a Python object.""" if not isinstance(protobuf_any, any_pb2.Any): raise ValueError(f"Unexpected type: {type(protobuf_any)}") From 83e63630bce34aec367453e6788553de2a6f6112 Mon Sep 17 00:00:00 2001 From: Brad Keryan Date: Wed, 21 May 2025 15:36:30 -0500 Subject: [PATCH 39/39] converters: Use type variables more consistently Signed-off-by: Brad Keryan --- src/nipanel/_converters.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/nipanel/_converters.py b/src/nipanel/_converters.py index 705368e..d1884a3 100644 --- a/src/nipanel/_converters.py +++ b/src/nipanel/_converters.py @@ -33,7 +33,7 @@ def protobuf_typename(self) -> str: """The protobuf name for the type.""" return self.protobuf_message.DESCRIPTOR.full_name # type: ignore[no-any-return] - def to_protobuf_any(self, python_value: Any) -> any_pb2.Any: + def to_protobuf_any(self, python_value: _TPythonType) -> any_pb2.Any: """Convert the Python object to its type-specific message and pack it as any_pb2.Any.""" message = self.to_protobuf_message(python_value) as_any = any_pb2.Any() @@ -41,7 +41,7 @@ def to_protobuf_any(self, python_value: Any) -> any_pb2.Any: return as_any @abstractmethod - def to_protobuf_message(self, python_value: Any) -> Message: + def to_protobuf_message(self, python_value: _TPythonType) -> _TProtobufType: """Convert the Python object to its type-specific message.""" def to_python(self, protobuf_value: any_pb2.Any) -> _TPythonType: @@ -70,7 +70,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.BoolValue]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.BoolValue - def to_protobuf_message(self, python_value: bool) -> Message: + def to_protobuf_message(self, python_value: bool) -> wrappers_pb2.BoolValue: """Convert the Python bool to a protobuf wrappers_pb2.BoolValue.""" return self.protobuf_message(value=python_value) @@ -92,7 +92,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.BytesValue]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.BytesValue - def to_protobuf_message(self, python_value: bytes) -> Message: + def to_protobuf_message(self, python_value: bytes) -> wrappers_pb2.BytesValue: """Convert the Python bytes string to a protobuf wrappers_pb2.BytesValue.""" return self.protobuf_message(value=python_value) @@ -114,7 +114,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.DoubleValue]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.DoubleValue - def to_protobuf_message(self, python_value: float) -> Message: + def to_protobuf_message(self, python_value: float) -> wrappers_pb2.DoubleValue: """Convert the Python float to a protobuf wrappers_pb2.DoubleValue.""" return self.protobuf_message(value=python_value) @@ -136,7 +136,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.Int64Value]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.Int64Value - def to_protobuf_message(self, python_value: int) -> Message: + def to_protobuf_message(self, python_value: int) -> wrappers_pb2.Int64Value: """Convert the Python int to a protobuf wrappers_pb2.Int64Value.""" return self.protobuf_message(value=python_value) @@ -158,7 +158,7 @@ def protobuf_message(self) -> Type[wrappers_pb2.StringValue]: """The type-specific protobuf message for the Python type.""" return wrappers_pb2.StringValue - def to_protobuf_message(self, python_value: str) -> Message: + def to_protobuf_message(self, python_value: str) -> wrappers_pb2.StringValue: """Convert the Python str to a protobuf wrappers_pb2.StringValue.""" return self.protobuf_message(value=python_value)