-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Fixing issues in the Jupyter notebook for the planner #873
Conversation
@microsoft-github-policy-service agree |
Thank you so much for creating this! This notebook needed to get updated to reflect the latest planner refactor. One comment from my side, for this particular example, could you show the output of the cells in the Jupyter notebook? Since it's a popular notebook that a lot of people like to look at (even on the Github website), it would be good to see what a "plan" looks like. Thanks again! @lemillermicrosoft @RogerBarreto can you take a look? |
@alexchaomander I included the plans. |
@kaza thanks for the PR, I'll leave some comments |
@kaza could you allow us to edit the PR of minor adjustments before merging? also, could you "Clear All Outputs" ? I'd be glad to do it but we cannot edit the PR |
I cleared all the outputs hover previously I was asked to inoclude them "could you show the output of the cells in the Jupyter notebook?". |
sorry for the churn @kaza -- yes we like showing the output but we cherry pick which cell, so rather than asking you which one and giving complex instructions I thought it would be easier to just drop the output, and we'll adjust it afterwards during the release process |
I would remove the custom serialization logic before merging, and leverage .NET JSON serializer + the fact that the Plan data structure is serializable |
ass suggested Co-authored-by: Devis Lucato <dluc@users.noreply.github.com>
### Motivation and Context This change fixes the issue in dotnet Jupyternotebooks, for the planner examples It useses different classes for the planer, and in addition it fixes few smaller issues regarding configuration and presenting of the resultes ### Description 1) Changed the used nuget version 2) Changed the old PlannerSkill class to SequentialPlanner 3) local config files, for azure and open AI, both must have all the properties as otherwise Json deserializer throws an exception, as I used manually changed config files, had issue in configuring both of them, so I added missing json properties 4) in the Planner originalPlan.Variables.ToPlan().PlanString has been removed so I created a new method in Utils and used that one
### Motivation and Context This change fixes the issue in dotnet Jupyternotebooks, for the planner examples It useses different classes for the planer, and in addition it fixes few smaller issues regarding configuration and presenting of the resultes ### Description 1) Changed the used nuget version 2) Changed the old PlannerSkill class to SequentialPlanner 3) local config files, for azure and open AI, both must have all the properties as otherwise Json deserializer throws an exception, as I used manually changed config files, had issue in configuring both of them, so I added missing json properties 4) in the Planner originalPlan.Variables.ToPlan().PlanString has been removed so I created a new method in Utils and used that one
Motivation and Context
This change fixes the issue in dotnet Jupyternotebooks, for the planner examples
It useses different classes for the planer, and in addition it fixes few smaller issues regarding configuration and presenting of the resultes
Description