From ed1fb084737d6d01f9e9215a92a50c7f378d3853 Mon Sep 17 00:00:00 2001 From: Evan Mattson <35585003+moonbox3@users.noreply.github.com> Date: Thu, 7 Nov 2024 11:08:12 -0500 Subject: [PATCH] Python: Add type hint for ProcessStepBuilder in send_event_to. Improve schema building. (#9608) ### Motivation and Context The `ProcessStepEdgeBuilder` `send_event_to` method allows one to pass in a target that isn't of type `ProcessFunctionTargetBuilder`. If it is of the other type `ProcessStepBuilder` we build the expected type `ProcessFunctionTargetBuilder`. The `send_event_to` method was missing the type hint, which can cause issues for type checkers. Secondly, during json schema building, we get the type hints using the builder's globals and locals. This is an issue because it doesn't allow us to find types defined outside of this module. Improve this by getting type hints for user defined modules, as well. ### Description This PR adds the proper type hint. - Closes #9605 - Closes #9606 - Renames a `test_postgres.py` file that existed both in unit tests and integration tests and caused a load conflict. ### Contribution Checklist - [X] The code builds clean without any errors or warnings - [X] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [X] All unit tests pass, and I have added new tests where possible - [X] I didn't break anyone :smile: --- .../processes/process_step_edge_builder.py | 18 ++++++-- .../schema/kernel_json_schema_builder.py | 5 ++- ...{test_postgres.py => test_postgres_int.py} | 0 .../test_process_step_edge_builder.py | 22 ++++++++++ .../tests/unit/services/test_service_utils.py | 44 +++++++++++++++++++ 5 files changed, 85 insertions(+), 4 deletions(-) rename python/tests/integration/memory/vector_stores/postgres/{test_postgres.py => test_postgres_int.py} (100%) diff --git a/python/semantic_kernel/processes/process_step_edge_builder.py b/python/semantic_kernel/processes/process_step_edge_builder.py index 05b395b3c904..4139540ea7e5 100644 --- a/python/semantic_kernel/processes/process_step_edge_builder.py +++ b/python/semantic_kernel/processes/process_step_edge_builder.py @@ -25,14 +25,26 @@ def __init__(self, source: "ProcessStepBuilder", event_id: str): self.source = source self.event_id = event_id - def send_event_to(self, target: ProcessFunctionTargetBuilder, **kwargs) -> "ProcessStepEdgeBuilder": - """Sends the event to the target.""" + def send_event_to( + self, target: "ProcessFunctionTargetBuilder | ProcessStepBuilder", **kwargs + ) -> "ProcessStepEdgeBuilder": + """Sends the event to the target. + + Args: + target: The target to send the event to. + **kwargs: Additional keyword arguments. + + Returns: + ProcessStepEdgeBuilder: The ProcessStepEdgeBuilder instance. + """ + from semantic_kernel.processes.process_step_builder import ProcessStepBuilder + if self.target is not None: raise ProcessInvalidConfigurationException( "An output target has already been set part of the ProcessStepEdgeBuilder." ) - if not isinstance(target, ProcessFunctionTargetBuilder): + if isinstance(target, ProcessStepBuilder): target = ProcessFunctionTargetBuilder(step=target, parameter_name=kwargs.get("parameter_name")) self.target = target diff --git a/python/semantic_kernel/schema/kernel_json_schema_builder.py b/python/semantic_kernel/schema/kernel_json_schema_builder.py index 9d19644fc25a..5ec519b5b377 100644 --- a/python/semantic_kernel/schema/kernel_json_schema_builder.py +++ b/python/semantic_kernel/schema/kernel_json_schema_builder.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. +import sys import types from enum import Enum from typing import Any, Union, get_args, get_origin, get_type_hints @@ -80,7 +81,9 @@ def build_model_schema( # https://github.com/microsoft/semantic-kernel/issues/6464 properties = {} required = [] - hints = get_type_hints(model, globals(), locals()) + + model_module_globals = vars(sys.modules[model.__module__]) + hints = get_type_hints(model, globalns=model_module_globals, localns={}) for field_name, field_type in hints.items(): field_description = None diff --git a/python/tests/integration/memory/vector_stores/postgres/test_postgres.py b/python/tests/integration/memory/vector_stores/postgres/test_postgres_int.py similarity index 100% rename from python/tests/integration/memory/vector_stores/postgres/test_postgres.py rename to python/tests/integration/memory/vector_stores/postgres/test_postgres_int.py diff --git a/python/tests/unit/processes/test_process_step_edge_builder.py b/python/tests/unit/processes/test_process_step_edge_builder.py index 43713373b72c..98f3396f6b5f 100644 --- a/python/tests/unit/processes/test_process_step_edge_builder.py +++ b/python/tests/unit/processes/test_process_step_edge_builder.py @@ -42,6 +42,28 @@ def test_send_event_to(): source.link_to.assert_called_once_with(event_id, edge_builder) +def test_send_event_to_step_builder_input(): + # Arrange + source = MagicMock(spec=ProcessStepBuilder) + source.link_to = MagicMock() + + target = MagicMock(spec=ProcessStepBuilder) + target.resolve_function_target = MagicMock( + return_value=MagicMock(function_name="mock_function_name", parameter_name="provided_parameter_name") + ) + + event_id = "event_003" + edge_builder = ProcessStepEdgeBuilder(source=source, event_id=event_id) + + # Act + edge_builder.send_event_to(target, parameter_name="provided_parameter_name") + + # Assert + assert edge_builder.target.step == target + assert edge_builder.target.parameter_name == "provided_parameter_name" + source.link_to.assert_called_once_with(event_id, edge_builder) + + def test_send_event_to_creates_target(): # Arrange source = MagicMock(spec=ProcessStepBuilder) diff --git a/python/tests/unit/services/test_service_utils.py b/python/tests/unit/services/test_service_utils.py index 1539577548a4..53ad67a761cc 100644 --- a/python/tests/unit/services/test_service_utils.py +++ b/python/tests/unit/services/test_service_utils.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. +import datetime from enum import Enum from typing import Annotated @@ -16,6 +17,16 @@ # region Test helpers +class DateTimePlugin: + class Data(KernelBaseModel): + timestamp: datetime.datetime + + @kernel_function(name="GetData", description="Get a Data class.") + def get_data(data: Annotated[Data, "The data."]) -> Annotated[Data, "The data."]: + """Get the data.""" + return data + + class BooleanPlugin: @kernel_function(name="GetBoolean", description="Get a boolean value.") def get_boolean(self, value: Annotated[bool, "The boolean value."]) -> Annotated[bool, "The boolean value."]: @@ -117,6 +128,7 @@ def setup_kernel(): "UnionPlugin": UnionTypePlugin(), "UnionPluginLegacy": UnionTypePluginLegacySyntax(), "EnumPlugin": EnumPlugin(), + "DateTimePlugin": DateTimePlugin(), }) return kernel @@ -396,3 +408,35 @@ def test_enum_plugin(setup_kernel): } assert complex_schema == expected_schema + + +def test_datetime_parameter(setup_kernel): + kernel = setup_kernel + + complex_func_metadata = kernel.get_list_of_function_metadata_filters( + filters={"included_plugins": ["DateTimePlugin"]} + ) + + complex_schema = kernel_function_metadata_to_function_call_format(complex_func_metadata[0]) + + expected_schema = { + "type": "function", + "function": { + "name": "DateTimePlugin-GetData", + "description": "Get a Data class.", + "parameters": { + "type": "object", + "properties": { + "data": { + "type": "object", + "properties": {"timestamp": {"type": "object"}}, + "required": ["timestamp"], + "description": "The data.", + } + }, + "required": ["data"], + }, + }, + } + + assert complex_schema == expected_schema