langchain icon indicating copy to clipboard operation
langchain copied to clipboard

[Azure] Centralizing Environment Validation for Azure OpenAI LLM & Chat Model & Embeddings Model

Open philipk19238 opened this issue 2 years ago • 4 comments

Description

References Issue #3769. Ports over functionality from AzureChatOpenAI to AzureOpenAIMixin to standardize interaction experience with Azure models.

Changes

  • Added AzureOpenAIMixin
  • Changed AzureOpenAI to inherit from AzureOpenAIMixin
  • Changed AzureChatOpenAI to inherit from AzureOpenAIMixin
  • Added relevant docstrings
  • Added AzureOpenAIEmbedding model
  • Added client type (openai.Completion...etc) validation

Testing

  • Ran on Jupyter Notebook

philipk19238 avatar Apr 29 '23 05:04 philipk19238

@hwchase17

philipk19238 avatar Apr 29 '23 19:04 philipk19238

can this also be used in embeddings?

Yes, it can. I'll adjust the PR tomorrow with an Azure class for embeddings. Might have to add a slight modification to the Mixin class to pass in the client (previous is openai.ChatCompletion) but embeddings is openai.Embedding.

philipk19238 avatar May 02 '23 03:05 philipk19238

can this also be used in embeddings?

Yes, it can. I'll adjust the PR tomorrow with an Azure class for embeddings. Might have to add a slight modification to the Mixin class to pass in the client (previous is openai.ChatCompletion) but embeddings is openai.Embedding.

well the LLM class should also not be ChatCompletion but Completion, right?

hwchase17 avatar May 02 '23 04:05 hwchase17

can this also be used in embeddings?

Yes, it can. I'll adjust the PR tomorrow with an Azure class for embeddings. Might have to add a slight modification to the Mixin class to pass in the client (previous is openai.ChatCompletion) but embeddings is openai.Embedding.

well the LLM class should also not be ChatCompletion but Completion, right?

Yes - good catch. Created a new commit that allocates the correct client per model type along with adding the AzureEmbeddings class.

philipk19238 avatar May 02 '23 20:05 philipk19238

@philipk19238 Hi , could you, please, resolve the merging issues and address the last comments (if needed)? After that, ping me and I push this PR for the review. Thanks!

leo-gan avatar Sep 13 '23 01:09 leo-gan

Closing because the PR wouldn't line up with the current directory structure of the library (would need to be in /libs/langchain/langchain instead of /langchain). Feel free to reopen against the current head if it's still relevant!

efriis avatar Nov 07 '23 04:11 efriis