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..53ad9a3 100644 --- a/examples/placeholder.py +++ b/examples/placeholder.py @@ -1 +1,58 @@ """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(str, enum.Enum): + """Example of a mixin string enum.""" + + VALUE1 = "value1" + VALUE2 = "value2" + VALUE3 = "value3" + + +if __name__ == "__main__": + my_panel = nipanel.StreamlitPanel( + panel_id="placeholder", + streamlit_script_uri=__file__, + ) + + 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/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/src/nipanel/_converters.py b/src/nipanel/_converters.py new file mode 100644 index 0000000..d1884a3 --- /dev/null +++ b/src/nipanel/_converters.py @@ -0,0 +1,210 @@ +"""Functions to convert between different data formats.""" + +from __future__ import annotations + +import logging +from abc import ABC, abstractmethod +from typing import Any, Generic, Type, TypeVar + +from google.protobuf import any_pb2, wrappers_pb2 +from google.protobuf.message import Message + +_TPythonType = TypeVar("_TPythonType") +_TProtobufType = TypeVar("_TProtobufType", bound=Message) + +_logger = logging.getLogger(__name__) + + +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]: + """The Python type that this converter handles.""" + + @property + @abstractmethod + def protobuf_message(self) -> Type[_TProtobufType]: + """The type-specific protobuf message for the Python type.""" + + @property + 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: _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() + as_any.Pack(message) + return as_any + + @abstractmethod + 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: + """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_value(self, protobuf_message: _TProtobufType) -> _TPythonType: + """Convert the protobuf wrapper message to its matching Python type.""" + + +class BoolConverter(Converter[bool, wrappers_pb2.BoolValue]): + """A converter for boolean types.""" + + @property + def python_type(self) -> Type[bool]: + """The Python type that this converter handles.""" + return bool + + @property + 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) -> wrappers_pb2.BoolValue: + """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: + """Convert the protobuf message to a Python bool.""" + return protobuf_value.value + + +class BytesConverter(Converter[bytes, wrappers_pb2.BytesValue]): + """A converter for byte string types.""" + + @property + def python_type(self) -> Type[bytes]: + """The Python type that this converter handles.""" + return bytes + + @property + 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) -> wrappers_pb2.BytesValue: + """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 a Python bytes string.""" + return protobuf_value.value + + +class FloatConverter(Converter[float, wrappers_pb2.DoubleValue]): + """A converter for floating point types.""" + + @property + def python_type(self) -> Type[float]: + """The Python type that this converter handles.""" + return float + + @property + 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) -> wrappers_pb2.DoubleValue: + """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: + """Convert the protobuf message to a Python float.""" + return protobuf_value.value + + +class IntConverter(Converter[int, wrappers_pb2.Int64Value]): + """A converter for integer types.""" + + @property + def python_type(self) -> Type[int]: + """The Python type that this converter handles.""" + return int + + @property + 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) -> wrappers_pb2.Int64Value: + """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: + """Convert the protobuf message to a Python int.""" + return protobuf_value.value + + +class StrConverter(Converter[str, wrappers_pb2.StringValue]): + """A converter for text string types.""" + + @property + def python_type(self) -> Type[str]: + """The Python type that this converter handles.""" + return str + + @property + 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) -> wrappers_pb2.StringValue: + """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: + """Convert the protobuf message to a Python string.""" + return protobuf_value.value + + +# FFV -- consider adding a RegisterConverter mechanism +_CONVERTIBLE_TYPES: list[Converter[Any, Any]] = [ + BoolConverter(), + BytesConverter(), + FloatConverter(), + IntConverter(), + StrConverter(), +] + +_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: + """Convert a Python object to a protobuf Any.""" + 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 + ) + if not best_matching_type: + raise TypeError( + 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}") + + converter = _CONVERTER_FOR_PYTHON_TYPE[best_matching_type] + return converter.to_protobuf_any(python_value) + + +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)}") + + underlying_typename = protobuf_any.TypeName() + _logger.debug(f"Unpacking type '{underlying_typename}'") + + converter = _CONVERTER_FOR_GRPC_TYPE[underlying_typename] + return converter.to_python(protobuf_any) 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/conftest.py b/tests/conftest.py index 580df51..1cb4284 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 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 @@ -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 new file mode 100644 index 0000000..0d17f9b --- /dev/null +++ b/tests/types.py @@ -0,0 +1,69 @@ +"""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): + """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 MixinIntEnum(int, enum.Enum): + """Example of an IntEnum using a mixin.""" + + VALUE11 = 11 + VALUE22 = 22 + VALUE33 = 33 + + +class MyStrEnum(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 3ffb5c2..0f2de89 100644 --- a/tests/unit/test_streamlit_panel.py +++ b/tests/unit/test_streamlit_panel.py @@ -1,5 +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 @@ -11,31 +13,114 @@ 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() - 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( 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() - 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 + + +def test___unopened_panel___set_value___sets_value( + fake_panel_channel: grpc.Channel, +) -> None: + """Test that set_value() succeeds before the user opens the panel.""" + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) + + value_id = "test_id" + string_value = "test_value" + + panel.set_value(value_id, string_value) + + +def test___unopened_panel___get_unset_value___raises_exception( + fake_panel_channel: grpc.Channel, +) -> None: + """Test that get_value() raises an exception for an unset value.""" + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) + + value_id = "test_id" + with pytest.raises(grpc.RpcError): + panel.get_value(value_id) + + +def test___unopened_panel___get_set_value___gets_value( + fake_panel_channel: grpc.Channel, +) -> None: + """Test that get_value() succeeds for a set value before the user opens the panel.""" + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) + + value_id = "test_id" + string_value = "test_value" + panel.set_value(value_id, string_value) + + assert panel.get_value(value_id) == string_value + + +@pytest.mark.parametrize( + "value_payload", + [ + "firstname bunchanumbers", + 42, + 3.14, + True, + b"robotext", + 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( + fake_panel_channel: grpc.Channel, + value_payload: object, +) -> None: + """Test that set_value() and get_value() work for builtin scalar types.""" + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) + + value_id = "test_id" + 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( + fake_panel_channel: grpc.Channel, + value_payload: object, +) -> None: + """Test that set_value() raises for unsupported types.""" + panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel) + + value_id = "test_id" + with pytest.raises(TypeError): + panel.set_value(value_id, value_payload) 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: diff --git a/tests/utils/_fake_python_panel_servicer.py b/tests/utils/_fake_python_panel_servicer.py index 7427400..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, @@ -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."""