Skip to content

Commit

Permalink
Python: Add type hint for ProcessStepBuilder in send_event_to. Improv…
Browse files Browse the repository at this point in the history
…e 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.

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### 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.

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [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 😄
  • Loading branch information
moonbox3 authored Nov 7, 2024
1 parent 1db12c4 commit ed1fb08
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 4 deletions.
18 changes: 15 additions & 3 deletions python/semantic_kernel/processes/process_step_edge_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion python/semantic_kernel/schema/kernel_json_schema_builder.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions python/tests/unit/processes/test_process_step_edge_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
44 changes: 44 additions & 0 deletions python/tests/unit/services/test_service_utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright (c) Microsoft. All rights reserved.

import datetime
from enum import Enum
from typing import Annotated

Expand All @@ -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."]:
Expand Down Expand Up @@ -117,6 +128,7 @@ def setup_kernel():
"UnionPlugin": UnionTypePlugin(),
"UnionPluginLegacy": UnionTypePluginLegacySyntax(),
"EnumPlugin": EnumPlugin(),
"DateTimePlugin": DateTimePlugin(),
})
return kernel

Expand Down Expand Up @@ -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

0 comments on commit ed1fb08

Please sign in to comment.