Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

.Net InputVariable.Default type is changed from string to object? #4345

Merged
merged 14 commits into from
Dec 18, 2023
Merged

.Net InputVariable.Default type is changed from string to object? #4345

merged 14 commits into from
Dec 18, 2023

Conversation

SergeyMenshykh
Copy link
Member

Motivation and Context

The SK is no longer string-centric and can now work with any .NET type, including strings. Functions can have parameters and return types of any type. Prompt variables can also be of any type. The last remaining element tied to the string type is the InputVariable.Default property. This PR addresses that.

Description

  • The InputVariable.Default property type changed from string to object.
  • All instances where the property is used are refactored accordingly. Where the string value of the property is required, it is converted to a string; in other cases, it is used as is.
  • Safeguards that prevent supplying default values of any type other than string in JSON and YAML prompt configurations are temporarily added until a proper, permanent solution is designed and implemented. More details below.

JsonSerializer & DeserializerBuilder handling primitives, objects, and arrays

Both JsonSerializer and DeserializerBuilder (from YamlDotNet) can infer the types of properties of the data model class into which they deserialize a string when a concrete, non-object type is specified. However, when the target type of a property is object, each attempts to accomplish deserialization differently.

The JsonSerializer, when deserializing to a property of object type, assigns a JsonElement (configurable, can be JsonNode) to the property, regardless of whether the type specified in the JSON is a primitive, array, or object.

The DeserializerBuilder deserializes all primitives as strings unless the !!type tag is used - !!int, !!bool, etc. As for complex types and arrays, those are deserialized as Dictionary<object, object> and List<object>, respectively.

Taking that into account, the solution should address the problem of having or using different types to represent the same data. The different types can be adapted to:

  • One universal data type for JSON/YAML objects (perhaps JsonElement?)
  • List<object> for JSON/YAML arrays
  • .NET primitives for JSON and YAML primitives - number, boolean, string

This way, function authors can write their functions regardless of whether the function is called from a JSON prompt or a YAML prompt.

Default value and schema

In addition to standard attributes/properties of function parameter metadata such as name, description, isRequired, and default, it's possible to specify a JSON schema to add extra information about the type of root and nested elements.

Ideally, the solution should take the schema into account, if specified, and use the type indicated there to convert the default value to at deserialization. In cases where the schema is not provided, the functionality should rely solely on the data types provided in JSON/YAML documents.

@SergeyMenshykh SergeyMenshykh requested a review from a team as a code owner December 17, 2023 23:41
@shawncal shawncal added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels Dec 17, 2023
@SergeyMenshykh SergeyMenshykh added the v1.0.1 Required for the Semantic Kernel v1.0.1 release label Dec 17, 2023
@SergeyMenshykh SergeyMenshykh mentioned this pull request Dec 18, 2023
Closed
@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Dec 18, 2023
Merged via the queue into microsoft:main with commit 3eaa507 Dec 18, 2023
18 checks passed
@SergeyMenshykh SergeyMenshykh deleted the default-type branch December 18, 2023 15:10
Kevdome3000 pushed a commit to Kevdome3000/semantic-kernel that referenced this pull request Dec 18, 2023
@SergeyMenshykh @Kevdome3000
.Net InputVariable.Default type is changed from string to object? (mi…
e7e01d3 
…crosoft#4345)

### Motivation and Context
The SK is no longer string-centric and can now work with any .NET type,
including strings. Functions can have parameters and return types of any
type. Prompt variables can also be of any type. The last remaining
element tied to the string type is the `InputVariable.Default` property.
This PR addresses that.

### Description
- The `InputVariable.Default` property type changed from string to
object.
- All instances where the property is used are refactored accordingly.
Where the string value of the property is required, it is converted to a
string; in other cases, it is used as is.
- Safeguards that prevent supplying default values of any type other
than string in JSON and YAML prompt configurations are temporarily added
until a proper, permanent solution is designed and implemented. More
details below.

### JsonSerializer & DeserializerBuilder handling primitives, objects,
and arrays

Both JsonSerializer and DeserializerBuilder (from YamlDotNet) can infer
the types of properties of the data model class into which they
deserialize a string when a concrete, non-object type is specified.
However, when the target type of a property is `object`, each attempts
to accomplish deserialization differently.

The JsonSerializer, when deserializing to a property of `object` type,
assigns a JsonElement (configurable, can be JsonNode) to the property,
regardless of whether the type specified in the JSON is a primitive,
array, or object.

The DeserializerBuilder deserializes all primitives as strings unless
the `!!type` tag is used - `!!int`, `!!bool`, etc. As for complex types
and arrays, those are deserialized as `Dictionary<object, object>` and
`List<object>`, respectively.

Taking that into account, the solution should address the problem of
having or using different types to represent the same data. The
different types can be adapted to:

- One universal data type for JSON/YAML objects (perhaps JsonElement?)
  - `List<object>` for JSON/YAML arrays
- .NET primitives for JSON and YAML primitives - number, boolean, string

This way, function authors can write their functions regardless of
whether the function is called from a JSON prompt or a YAML prompt.

### Default value and schema

In addition to standard attributes/properties of function parameter
metadata such as name, description, isRequired, and default, it's
possible to specify a [JSON schema](https://json-schema.org/) to add
extra information about the type of root and nested elements.

Ideally, the solution should take the schema into account, if specified,
and use the type indicated there to convert the default value to at
deserialization. In cases where the schema is not provided, the
functionality should rely solely on the data types provided in JSON/YAML
documents.

(cherry picked from commit 3eaa507)
Kevdome3000 pushed a commit to Kevdome3000/semantic-kernel that referenced this pull request Dec 18, 2023
@SergeyMenshykh @Kevdome3000
.Net InputVariable.Default type is changed from string to object? (mi…
2e0bb8e 
…crosoft#4345)

### Motivation and Context
The SK is no longer string-centric and can now work with any .NET type,
including strings. Functions can have parameters and return types of any
type. Prompt variables can also be of any type. The last remaining
element tied to the string type is the `InputVariable.Default` property.
This PR addresses that.

### Description
- The `InputVariable.Default` property type changed from string to
object.
- All instances where the property is used are refactored accordingly.
Where the string value of the property is required, it is converted to a
string; in other cases, it is used as is.
- Safeguards that prevent supplying default values of any type other
than string in JSON and YAML prompt configurations are temporarily added
until a proper, permanent solution is designed and implemented. More
details below.

### JsonSerializer & DeserializerBuilder handling primitives, objects,
and arrays

Both JsonSerializer and DeserializerBuilder (from YamlDotNet) can infer
the types of properties of the data model class into which they
deserialize a string when a concrete, non-object type is specified.
However, when the target type of a property is `object`, each attempts
to accomplish deserialization differently.

The JsonSerializer, when deserializing to a property of `object` type,
assigns a JsonElement (configurable, can be JsonNode) to the property,
regardless of whether the type specified in the JSON is a primitive,
array, or object.

The DeserializerBuilder deserializes all primitives as strings unless
the `!!type` tag is used - `!!int`, `!!bool`, etc. As for complex types
and arrays, those are deserialized as `Dictionary<object, object>` and
`List<object>`, respectively.

Taking that into account, the solution should address the problem of
having or using different types to represent the same data. The
different types can be adapted to:

- One universal data type for JSON/YAML objects (perhaps JsonElement?)
  - `List<object>` for JSON/YAML arrays
- .NET primitives for JSON and YAML primitives - number, boolean, string

This way, function authors can write their functions regardless of
whether the function is called from a JSON prompt or a YAML prompt.

### Default value and schema

In addition to standard attributes/properties of function parameter
metadata such as name, description, isRequired, and default, it's
possible to specify a [JSON schema](https://json-schema.org/) to add
extra information about the type of root and nested elements.

Ideally, the solution should take the schema into account, if specified,
and use the type indicated there to convert the default value to at
deserialization. In cases where the schema is not provided, the
functionality should rely solely on the data types provided in JSON/YAML
documents.

(cherry picked from commit 3eaa507)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephentoub stephentoub stephentoub approved these changes

@RogerBarreto RogerBarreto RogerBarreto approved these changes

@markwallace-microsoft markwallace-microsoft markwallace-microsoft approved these changes

@dmytrostruk dmytrostruk Awaiting requested review from dmytrostruk

Assignees
No one assigned
Labels
kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code v1.0.1 Required for the Semantic Kernel v1.0.1 release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@SergeyMenshykh @stephentoub @RogerBarreto @markwallace-microsoft @shawncal