Skip to content

Python: add parsing of (Azure) OpenAI into classes, including ChatMessages - #2442

Closed
Eduard van Valkenburg (eavanvalkenburg) wants to merge 9 commits into
microsoft:mainfrom
eavanvalkenburg:chat_message
Closed

Eduard van Valkenburg (eavanvalkenburg) wants to merge 9 commits into
microsoft:mainfrom
eavanvalkenburg:chat_message

Conversation

@eavanvalkenburg

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) commented Aug 14, 2023 •

Copy link
Copy Markdown
Member

Motivation and Context

  1. There were a lot of opinions in what is and isn't returned by the different OpenAI classes, that is now much more complete (things like Usage are returned as well)
  2. Tricks had to be done to deal with 1 or more results in the openai class, that now moves to downstream classes, or user code.
  3. This will also make Function Calling much easier
  4. Adds Usage on both service and function level
  5. This will make it much easier to store and restore chat messages

One note, most of the completion results and other classes are based on the openai api definitions, which are probably the most prevalent, but a mapping of these and HF and other ai services is needed.

Description

  1. Added models folder to store completion results, usage_result, finish_reason, completion_content and appropriate base classes as well as the chat versions of these, and a Role class.
  2. Added models into connectors\ai\openai\models to store subclasses of above, with specific classmethods that create these items from openai_objects.
  3. Adapted chat_completion_client_base and text_completion_client_base to have ai_service_client_base as the base which houses a logger and usage, adapted all downstream methods for these objects (complete_chat_async and the like)
  4. adapted tests for all this and added unit tests for all new classes (except enums)

Contribution Checklist

@lemillermicrosoft

Copy link
Copy Markdown
Member

Marking as draft while this is broken down into smaller pieces. We should also make there is an accompanying issue to make sure we maintain parity across .Net and python.

@lemillermicrosoft
Lee Miller (lemillermicrosoft) marked this pull request as draft August 24, 2023 18:37
@eavanvalkenburg

Copy link
Copy Markdown
Member Author

Makes sense Lee Miller (@lemillermicrosoft) it did get a bit big. I did hear there were already pieces of this (using a object, rather then a tuple for the response) in the .net version, but did one did go a lot further, should I create the issue for this or will you?

@lemillermicrosoft

Copy link
Copy Markdown
Member

Thanks for your PR and contribution. For now, we are closing old drafts. Please re-open as appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants