-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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.
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
.Net: [OpenAPI] Removing duplication in parameters serialization functionality #2923
Merged
SergeyMenshykh
merged 1 commit into
microsoft:main
from
SergeyMenshykh:openapi-improvements-for-array-parameter-serialization
Sep 22, 2023
Merged
.Net: [OpenAPI] Removing duplication in parameters serialization functionality #2923
SergeyMenshykh
merged 1 commit into
microsoft:main
from
SergeyMenshykh:openapi-improvements-for-array-parameter-serialization
Sep 22, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…capsulated in the `ArrayParameterValueSerializer` class.
SergeyMenshykh
requested review from
dmytrostruk,
RogerBarreto and
markwallace-microsoft
September 21, 2023 15:06
SergeyMenshykh
added
the
PR: ready for review
All feedback addressed, ready for reviews
label
Sep 21, 2023
lemillermicrosoft
approved these changes
Sep 21, 2023
markwallace-microsoft
approved these changes
Sep 22, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
...unctions/Functions.UnitTests/OpenAPI/Builders/Serialization/ArrayParameterSerializerTests.cs
Show resolved
Hide resolved
SergeyMenshykh
deleted the
openapi-improvements-for-array-parameter-serialization
branch
September 22, 2023 09:05
lemillermicrosoft
added a commit
to gitri-ms/semantic-kernel
that referenced
this pull request
Sep 23, 2023
commit 2df1499 Author: Lee Miller <lemiller@microsoft.com> Date: Fri Sep 22 15:39:56 2023 -0700 .Net: Refactor PlannerConfig classes for better organization (microsoft#2912) Moved common properties and methods from SequentialPlannerConfig and StepwisePlannerConfig to the base class PlannerConfigBase. This change improves code organization and reduces redundancy. Additionally, added a new GetSkillFunction property to the PlannerConfigBase class, allowing for custom function lookup behavior. This change provides flexibility in how skill functions are retrieved and used within the planning process. Resolves microsoft#2911 <!-- 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. --> <!-- Describe your changes, the overall approach, the underlying design. These notes will help understanding how your code works. Thanks! --> <!-- 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 😄 commit 4a2cf70 Author: Lee Miller <lemiller@microsoft.com> Date: Fri Sep 22 12:07:27 2023 -0700 .Net: Add new token counter implementations to TextChunker (microsoft#2840) Implement MicrosoftML and DeepDev token counters in the TextChunker example. Update the project file with new package references and modify the RunExampleWithCustomTokenCounter method to support different token counter types. Inspired by microsoft#2809 Fixes microsoft#478 | Iteration | MicrosoftML (ms) | MicrosoftMLRoberta (ms) | SharpToken (ms) | DeepDev (ms) | |------------|-------------------|--------------------------|-----------------|--------------| | 1 | 38 | 10,189 | 14,305 | 16,701 | | 2 | 36 | 5,581 | 8,381 | 14,214 | | 3 | 13 | 5,354 | 7,955 | 13,630 | | 4 | 27 | 5,679 | 9,156 | 16,164 | | 5 | 16 | 5,158 | 8,657 | 17,276 | | Average | 26.0 | 7,512.2 | 9,710.8 | 15,597 | <sup style="font-size: smaller;">(Avg. Execution Time: 9,710.8 ms)</sup> ``` The city of Venice, located in the northeastern part of Italy, is renowned for its unique geographical features. Built on more than 100 small islands in a lagoon in the Adriatic Sea, it has no roads, just canals including the Grand Canal thoroughfare lined with Renaissance and Gothic palaces. The central square, Piazza San Marco, contains St. Mark's Basilica, which is tiled with Byzantine mosaics, and the Campanile bell tower offering views of the city's red roofs. ------------------------ The Amazon Rainforest, also known as Amazonia, is a moist broadleaf tropical rainforest in the Amazon biome that covers most of the Amazon basin of South America. This basin encompasses 7 million square kilometers, of which 5.5 million square kilometers are covered by the rainforest. This region includes territory belonging to nine nations and 3.4 million square kilometers of uncontacted tribes. The Amazon represents over half of the planet's remaining rainforests and comprises the largest and most biodiverse tract of tropical rainforest in the world. ------------------------ The Great Barrier Reef is the world's largest coral reef system composed of over 2,900 individual reefs and 900 islands stretching for over 2,300 kilometers over an area of approximately 344,400 square kilometers. The reef is located in the Coral Sea, off the coast of Queensland, Australia. The Great Barrier Reef can be seen from outer space and is the world's biggest single structure made by living organisms. This reef structure is composed of and built by billions of tiny organisms, known as coral polyps. ``` <sup style="font-size: smaller;">(Avg. Execution Time: 26.0 ms)</sup> ``` The city of Venice, located in the northeastern part of Italy, is renowned for its unique geographical features. Built on more than 100 small ------------------------ islands in a lagoon in the Adriatic Sea, it has no roads, just canals including the Grand Canal thoroughfare lined with Renaissance and ------------------------ Gothic palaces. The central square, Piazza San Marco, contains St. Mark's Basilica, which is tiled with Byzantine mosaics, ------------------------ and the Campanile bell tower offering views of the city's red roofs. The Amazon Rainforest, also known as Amazonia, ------------------------ is a moist broadleaf tropical rainforest in the Amazon biome that covers most of the Amazon basin of South America. This basin encompasses 7 ------------------------ million square kilometers, of which 5. 5 million square kilometers are covered by the rainforest. This region includes territory ------------------------ belonging to nine nations and 3. 4 million square kilometers of uncontacted tribes. The Amazon represents over ------------------------ half of the planet's remaining rainforests and comprises the largest and most biodiverse tract of tropical rainforest in the world. ------------------------ The Great Barrier Reef is the world's largest coral reef system composed of over 2, 900 individual reefs and 900 islands ------------------------ stretching for over 2, 300 kilometers over an area of approximately 344, 400 square kilometers. The reef is located in the ------------------------ Coral Sea, off the coast of Queensland, Australia. The Great Barrier Reef can be seen from outer space and is the world's ------------------------ biggest single structure made by living organisms. This reef structure is composed of and built by billions of tiny organisms, known as coral polyps. ``` <sup style="font-size: smaller;">(Avg. Execution Time: 7,512.2 ms)</sup> ``` The city of Venice, located in the northeastern part of Italy, is renowned for its unique geographical features. Built on more than 100 small islands in a lagoon in the Adriatic Sea, it has no roads, just canals including the Grand Canal thoroughfare lined with Renaissance and Gothic palaces. The central square, Piazza San Marco, contains St. Mark's Basilica, which is tiled with Byzantine mosaics, and the Campanile bell tower offering views of the city's red roofs. ------------------------ The Amazon Rainforest, also known as Amazonia, is a moist broadleaf tropical rainforest in the Amazon biome that covers most of the Amazon basin of South America. This basin encompasses 7 million square kilometers, of which 5.5 million square kilometers are covered by the rainforest. This region includes territory belonging to nine nations and 3.4 million square kilometers of uncontacted tribes. The Amazon represents over half of the planet's remaining rainforests and comprises the largest and most biodiverse tract of tropical rainforest in the world. ------------------------ The Great Barrier Reef is the world's largest coral reef system composed of over 2,900 individual reefs and 900 islands stretching for over 2,300 kilometers over an area of approximately 344,400 square kilometers. The reef is located in the Coral Sea, off the coast of Queensland, Australia. The Great Barrier Reef can be seen from outer space and is the world's biggest single structure made by living organisms. This reef structure is composed of and built by billions of tiny organisms, known as coral polyps. ``` <sup style="font-size: smaller;">(Avg. Execution Time: 15,597 ms)</sup> ``` The city of Venice, located in the northeastern part of Italy, is renowned for its unique geographical features. Built on more than 100 small islands in a lagoon in the Adriatic Sea, it has no roads, just canals including the Grand Canal thoroughfare lined with Renaissance and Gothic palaces. The central square, Piazza San Marco, contains St. Mark's Basilica, which is tiled with Byzantine mosaics, and the Campanile bell tower offering views of the city's red roofs. ------------------------ The Amazon Rainforest, also known as Amazonia, is a moist broadleaf tropical rainforest in the Amazon biome that covers most of the Amazon basin of South America. This basin encompasses 7 million square kilometers, of which 5.5 million square kilometers are covered by the rainforest. This region includes territory belonging to nine nations and 3.4 million square kilometers of uncontacted tribes. The Amazon represents over half of the planet's remaining rainforests and comprises the largest and most biodiverse tract of tropical rainforest in the world. ------------------------ The Great Barrier Reef is the world's largest coral reef system composed of over 2,900 individual reefs and 900 islands stretching for over 2,300 kilometers over an area of approximately 344,400 square kilometers. The reef is located in the Coral Sea, off the coast of Queensland, Australia. The Great Barrier Reef can be seen from outer space and is the world's biggest single structure made by living organisms. This reef structure is composed of and built by billions of tiny organisms, known as coral polyps. ``` <!-- Before submitting this PR, please make sure: --> - [ ] The code builds clean without any errors or warnings - [ ] 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 - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄 commit 544b6c1 Author: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com> Date: Fri Sep 22 16:37:57 2023 +0100 .Net: New result types - FunctionResult and KernelResult (microsoft#2864) <!-- 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. --> This PR contains changes to replace `SKContext` as result type with new types `FunctionResult` and `KernelResult`. 1. `FunctionResult` - new return type from single function invocation. It contains `object? Value` which can be any primitive, complex type or `IAsyncEnumerable<T>`. It also contains `SKContext`, but internally. This is required to pass `SKContext` to the next function in pipeline and keep the implementation simple at first iteration. Context could be removed from `FunctionResult` if needed in the future. It's not publicly available as part of `FunctionResult`. 2. `KernelResult` - new return type in `Kernel.RunAsync` method. It doesn't contain `SKContext`, just `object? Value` to get execution result. `FunctionResult` also contains `Metadata` - property, that contains additional data, including AI model response (e.g. token usage). `KernelResult` contains collection of `FunctionResult` - all function results from pipeline, so it's possible to check result of each function in pipeline, observe result value, AI-related information, like amount of tokens etc. Syntax in examples and tests were changed from `result.Result` to `result.GetValue<string>`, but now it should be possible to get any type as a result and not only string. <!-- Describe your changes, the overall approach, the underlying design. These notes will help understanding how your code works. Thanks! --> 1. Updated `ISKFunction` interface to return `FunctionResult` instead of `SKContext`. 2. Updated `IKernel` interface to return `KernelResult` instead of `SKContext`. 3. Updated `NativeFunction` to support `IAsyncEnumerable<T>` return type. 4. Added tests to `SKFunctionTests2` to verify the behavior for different result types. 5. Removed test `KernelTests.ItProvidesAccessToFunctionsViaSKContextAsync`. Reason: Invalid scenario. Kernel should not provide public access to functions via `SKContext`. 6. Updated tests and examples across the codebase to align with new signature. <!-- 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 - [ ] I didn't break anyone 😄 - BREAKING CHANGE: This PR will modify syntax of getting result using `ISKFunction.InvokeAsync` and `IKernel.RunAsync` methods. Syntax changed from `SKContext.Result` to `FunctionResult/KernelResult.GetValue<T>()`. commit bf9c1fc Author: Mark Wallace <127216156+markwallace-microsoft@users.noreply.github.com> Date: Fri Sep 22 14:38:13 2023 +0100 .Net: Rename 'Skills' -> 'Plugins' - Part 8 Update plugin names, descriptions, and parameters (microsoft#2940) - [ ] The code builds clean without any errors or warnings - [ ] 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 - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄 This pull request updates plugin names, descriptions, and parameters in the Semantic Kernel project. Specifically, it updates the descriptions for several connectors and planners to refer to Semantic Kernel plugins instead of skills, and renames the 'FunSkill' plugin to 'FunPlugin'. Additionally, it updates the 'executionParameters' parameter to 'functionExecutionParameters'. - Updated descriptions for Chroma, DuckDB, Milvus, Pinecone, Postgres, Qdrant, Redis, SQLite, and Weaviate connectors to refer to Semantic Kernel plugins instead of skills - Updated SequentialPlanner and ActionPlanner to exclude plugins instead of skills - Renamed 'FunSkill' plugin to 'FunPlugin' in OpenAPI documents - Renamed 'executionParameters' parameter to 'functionExecutionParameters' in OpenAPI documents --- *Powered by [Microsoft Semantic Kernel](https://github.com/microsoft/semantic-kernel)* commit 48b6e2a Author: Mark Wallace <127216156+markwallace-microsoft@users.noreply.github.com> Date: Fri Sep 22 13:57:00 2023 +0100 .Net: Rename 'Skills' -> 'Plugins' - Part 7 Update .md files terminology from SKILLS to PLUGINS (microsoft#2939) Replace `Skill` with `Plugin` in docs and dotnet readme's - [ ] The code builds clean without any errors or warnings - [ ] 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 - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄 This set of commits updates the terminology used in the project from SKILLS to PLUGINS. SKILLS have been renamed to PLUGINS throughout the codebase, and related documentation and comments have been updated accordingly. Additionally, the telemetry metrics have been updated to reflect the new naming convention. - Rename SKILLS to PLUGINS throughout the codebase - Update related documentation and comments - Update telemetry metrics to reflect new naming convention --- *Powered by [Microsoft Semantic Kernel](https://github.com/microsoft/semantic-kernel)* commit ef4c5a5 Author: Mark Wallace <127216156+markwallace-microsoft@users.noreply.github.com> Date: Fri Sep 22 11:24:42 2023 +0100 .Net: Rename 'Skills' -> 'Plugins' - Part 6 Updates to MsGraphSkillsExample, OpenApiPluginsExample, and graph-api-skills projects (microsoft#2938) - Rename MsGraphSkillsExample - Rename OpenApiSkillsExample - [ ] The code builds clean without any errors or warnings - [ ] 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 - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄 This pull request includes updates to three different projects. In the MsGraphSkillsExample project, the name has been changed to MsGraphPluginsExample to better reflect the nature of the project as a set of plugins rather than skills. In the OpenApiPluginsExample project, the OpenAPI skills have been renamed to OpenAPI plugins throughout the project, and the GitHub plugin has been updated to use a new API key configuration option. - Renamed MsGraphSkillsExample project to MsGraphPluginsExample - Updated all relevant files and references to reflect the new project name - Changed references to 'skills' to 'plugins' in RepoFiles.cs - Updated README.md to reflect the new project name and purpose - Changed exception message in YourAppException.cs to reflect the new project name - Renamed AzureOpenAIConfiguration.cs, OpenAIConfiguration.cs, Program.cs, and README.md to reflect the new project name and purpose - Updated the OpenApiSkillsExample project to use the new GitHubPlugin instead of the deprecated GitHubSkill. - Renamed the 'openapi-skills' directory to 'OpenApiPluginsExample'. - Updated the README.md file to reflect the changes and to use the term 'plugin' instead of 'skill'. - Renamed the 'RequiredOnPropertyValueAttribute.cs' file to 'OpenApiPluginsExample'. - Updated the 'appsettings.json' file to use the new GitHubPlugin options. --- *Powered by [Microsoft Semantic Kernel](https://github.com/microsoft/semantic-kernel)* commit 559d6f3 Author: Mark Wallace <127216156+markwallace-microsoft@users.noreply.github.com> Date: Fri Sep 22 10:38:38 2023 +0100 .Net: Rename the folder containing the SemanticKernel.Core project (microsoft#2919) Rename the folder containing the SemanticKernel.Core project to match the project name - [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 😄 --------- Co-authored-by: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> commit 4d7dfe3 Author: Anthony Puppo <anthonyosx@gmail.com> Date: Fri Sep 22 05:36:30 2023 -0400 .Net: Stream response should not skip white space (microsoft#2922) The current Azure OpenAI streaming implementation skips white space (ie. space character token ID 220). All non-empty tokens should be streamed, including space characters. This is rare as most tokens are prefixed with a space by default, but still an issue in some cases (such as formatted markdown, code, etc). Fixes microsoft#2921 Modify the streaming logic to check for null or empty instead of null, empty and white space. <!-- 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 😄 Co-authored-by: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> commit 810627a Author: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> Date: Fri Sep 22 09:58:51 2023 +0100 .Net: [OpenAPI] Removing duplication in parameters serialization functionality (microsoft#2923) The `FormStyleParameterSerializer` and `SpaceDelimitedStyleParameterSerializer` classes currently share similar serialization logic for array-type parameters. The upcoming PR introduces the `PipeDelimitedStyleParameterSerializer` class, which adds a similar but slightly different serialization functionality. To prevent code duplication, the common serialization logic is moved to the `ArrayParameterValueSerializer` class. Related issue - microsoft#2745 - The `ArrayParameterValueSerializer` class has been added to contain common serialization functionality for parameters of an array type. - The `FormStyleParameterSerializer` and `SpaceDelimitedStyleParameterSerializer` classes have been modified to delegate array parameter serialization to the `ArrayParameterValueSerializer` class. <!-- 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 😄 commit e3868df Author: Mark Wallace <127216156+markwallace-microsoft@users.noreply.github.com> Date: Fri Sep 22 07:08:23 2023 +0100 .Net: Rename 'Skills' -> 'Plugins' - Part 5 (microsoft#2918) - Update samples to use plugins and not skills - Update integration tests to use plugins and not skills - Switch to using the sample semantic plugins - [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 😄 --------- Co-authored-by: Lee Miller <lemiller@microsoft.com> commit 283a0de Author: Anthony Puppo <anthonyosx@gmail.com> Date: Thu Sep 21 18:35:49 2023 -0400 .Net: Update AggregatePartitionedResultsAsync method definition with additional arguments (microsoft#2731) The existing `AggregatePartitionedResultsAsync` extension method doesn't expose common arguments used when invoking functions and makes an assumption on how to separate results. Update the `AggregatePartitionedResultsAsync` method definition with additional arguments to both utilize and proxy through to `InvokeAsync`. <!-- 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 😄 --------- Co-authored-by: Lee Miller <lemiller@microsoft.com>
SOE-YoungS
pushed a commit
to SOE-YoungS/semantic-kernel
that referenced
this pull request
Nov 1, 2023
…tionality (microsoft#2923) ### Motivation, Context The `FormStyleParameterSerializer` and `SpaceDelimitedStyleParameterSerializer` classes currently share similar serialization logic for array-type parameters. The upcoming PR introduces the `PipeDelimitedStyleParameterSerializer` class, which adds a similar but slightly different serialization functionality. To prevent code duplication, the common serialization logic is moved to the `ArrayParameterValueSerializer` class. Related issue - microsoft#2745 ### Description - The `ArrayParameterValueSerializer` class has been added to contain common serialization functionality for parameters of an array type. - The `FormStyleParameterSerializer` and `SpaceDelimitedStyleParameterSerializer` classes have been modified to delegate array parameter serialization to the `ArrayParameterValueSerializer` class. ### 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 😄
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
.NET
Issue or Pull requests regarding .NET code
PR: ready for review
All feedback addressed, ready for reviews
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation, Context
The
FormStyleParameterSerializer
andSpaceDelimitedStyleParameterSerializer
classes currently share similar serialization logic for array-type parameters. The upcoming PR introduces thePipeDelimitedStyleParameterSerializer
class, which adds a similar but slightly different serialization functionality. To prevent code duplication, the common serialization logic is moved to theArrayParameterValueSerializer
class.Related issue - #2745
Description
ArrayParameterValueSerializer
class has been added to contain common serialization functionality for parameters of an array type.FormStyleParameterSerializer
andSpaceDelimitedStyleParameterSerializer
classes have been modified to delegate array parameter serialization to theArrayParameterValueSerializer
class.Contribution Checklist