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

Python: Add sequential planner to python #1916

Merged
merged 57 commits into from
Jul 28, 2023
Merged

Python: Add sequential planner to python #1916

merged 57 commits into from
Jul 28, 2023

Conversation

joowon-dm-snu
Copy link
Contributor

@joowon-dm-snu joowon-dm-snu commented Jul 9, 2023 •
edited by nacharya1
Loading

Motivation and Context

fixes #1915

Description

Contribution Checklist

@shawncal shawncal added the python Pull requests for the Python Semantic Kernel label Jul 9, 2023
@awharrison-28
Copy link
Contributor

@joowon-dm-snu I addressed the linting errors and made a few small changes to address the unit tests failing (downstream affect from the bug you fixed) - if you're good with the changes, go ahead and resolve the conversation I added.

LGTM - would love to get this in :)

awharrison-28
awharrison-28 previously approved these changes Jul 25, 2023
View reviewed changes
@joowon-dm-snu
Copy link
Contributor Author

joowon-dm-snu commented Jul 26, 2023 •
edited
Loading

@awharrison-28
made 2 changes.

  1. fix wrong iteration
  2. sync planner to Add support for multiple native function arguments of many types #1195 - this PR updates planner object

@joowon-dm-snu
Copy link
Contributor Author

joowon-dm-snu commented Jul 26, 2023 •
edited
Loading

@awharrison-28
and i think planner object still need more improvements on its logic.
However, since there are no issues with the sequential planner itself, it would be beneficial to thoroughly examine it in another issue after merging this PR

@awharrison-28 awharrison-28 mentioned this pull request Jul 26, 2023
Merged
7 tasks
awharrison-28
awharrison-28 previously approved these changes Jul 28, 2023
View reviewed changes
Copy link
Contributor

@awharrison-28 awharrison-28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the input variable checking - works much better now and changed the syntax example prompt to something that runs more reliably - there are some issues around variable typing for mathskill that should be addressed in another PR.

@awharrison-28 awharrison-28 added this pull request to the merge queue Jul 28, 2023
Merged via the queue into microsoft:main with commit 484b39d Jul 28, 2023
20 checks passed
SOE-YoungS pushed a commit to SOE-YoungS/semantic-kernel that referenced this pull request Nov 1, 2023
@joowon-dm-snu @awharrison-28
@shawncal @SOE-YoungS
Python: Add sequential planner to python (microsoft#1916)
12e971d 
### Motivation and Context
<!-- 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.
-->
fixes microsoft#1915 

### Description
<!-- 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: -->
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [ ] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄

---------

Co-authored-by: Abby Harrison <abby.harrison@microsoft.com>
Co-authored-by: Abby Harrison <54643756+awharrison-28@users.noreply.github.com>
Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
@joowon-dm-snu @awharrison-28 @shawncal
Python: Add sequential planner to python (microsoft#1916)
2a5e28b 
### Motivation and Context
<!-- 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.
-->
fixes microsoft#1915 

### Description
<!-- 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: -->
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [ ] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄

---------

Co-authored-by: Abby Harrison <abby.harrison@microsoft.com>
Co-authored-by: Abby Harrison <54643756+awharrison-28@users.noreply.github.com>
Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
@joowon-dm-snu @awharrison-28 @shawncal
Python: Add sequential planner to python (microsoft#1916)
5ed5418 
### Motivation and Context
<!-- 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.
-->
fixes microsoft#1915 

### Description
<!-- 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: -->
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [ ] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄

---------

Co-authored-by: Abby Harrison <abby.harrison@microsoft.com>
Co-authored-by: Abby Harrison <54643756+awharrison-28@users.noreply.github.com>
Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awharrison-28 awharrison-28 awharrison-28 approved these changes

Assignees

@mkarle mkarle

@awharrison-28 awharrison-28

Labels
PR: feedback to address Waiting for PR owner to address comments/questions python Pull requests for the Python Semantic Kernel
Projects
Semantic Kernel
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Python: Add sequential planner to python
5 participants
@joowon-dm-snu @alexchaomander @awharrison-28 @mkarle @shawncal