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.

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

Merge KernelConfig with Kernel #731

Merged
merged 13 commits into from May 6, 2023
Merged

Merge KernelConfig with Kernel #731

merged 13 commits into from May 6, 2023

Conversation

mkarle
Copy link
Contributor

@mkarle mkarle commented Apr 28, 2023

Motivation and Context

KernelConfig isn't needed as it's less pythonic and is not used independently from Kernel. This moves all its methods and members into Kernel.

Description

  • Removed KernelConfig
  • Put KernelConfig's methods and variables into Kernel
  • Ran unit and end-to-end tests
  • Did not run notebooks because they are out of date with the current repo.

Contribution Checklist

@github-actions github-actions bot added the python Pull requests for the Python Semantic Kernel label Apr 28, 2023
@mkarle mkarle added the PR: ready for review All feedback addressed, ready for reviews label May 1, 2023
awharrison-28
awharrison-28 previously approved these changes May 5, 2023
@awharrison-28
Copy link
Contributor

Looks like there are conflicts to resolve in notebook 5, otherwise, LGTM

shawncal
shawncal previously approved these changes May 6, 2023
@shawncal shawncal merged commit 3e1d13e into microsoft:main May 6, 2023
20 checks passed
@nsteblay
Copy link

nsteblay commented May 7, 2023

I'm far from a strong Python engineer but have been cloning this repo to learn about the Semantic Kernel. This change breaks all the Python notebooks. To continue my learning I need to reverse the changes that were made. I guess the notebooks aren't as "Pythonic" as they should be, whatever that means, but at least they worked. Given that this is code for introducing some of the basics I would highly recommend that the code work. This change provides no value at all, exactly the opposite.

codebrain pushed a commit to searchpioneer/semantic-kernel that referenced this pull request May 16, 2023
### Motivation and Context
KernelConfig isn't needed as it's less pythonic and is not used
independently from Kernel. This moves all its methods and members into
Kernel.

### Description
- Removed KernelConfig
- Put KernelConfig's methods and variables into Kernel
- Ran unit and end-to-end tests
- Did not run notebooks because they are out of date with the current
repo.

---------
Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
### Motivation and Context
KernelConfig isn't needed as it's less pythonic and is not used
independently from Kernel. This moves all its methods and members into
Kernel.

### Description
- Removed KernelConfig
- Put KernelConfig's methods and variables into Kernel
- Ran unit and end-to-end tests
- Did not run notebooks because they are out of date with the current
repo.

---------
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
Labels
PR: ready for review All feedback addressed, ready for reviews python Pull requests for the Python Semantic Kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants