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

Support multiple openai versions in python #27

Merged
merged 6 commits into from
Nov 10, 2023
Merged

Support multiple openai versions in python #27

merged 6 commits into from
Nov 10, 2023

Conversation

ankrgyl
Copy link
Contributor

@ankrgyl ankrgyl commented Nov 9, 2023

This change updates autoevals to support both the v0 and v1 versions of OpenAI's python library. To accomplish this, we:

  • Remove openai as a dependency. This enables autoevals to be included flexibly as a dependency in systems which use either version of openai.
  • Wrap the completion method and rate limit exception in a way that works for either SDK version
  • Switch to using the wrappers built into the braintrust sdk instead of a custom one (except for cached requests...).

Tested using end-to-end scripts with both versions of python.

ankrgyl added a commit to braintrustdata/braintrust-sdk that referenced this pull request Nov 9, 2023
@ankrgyl ankrgyl requested a review from manugoyal November 9, 2023 19:18
@@ -18,7 +21,7 @@ jobs:
python-version: "3.11"
- name: Install dependencies
run: |
python -m pip install --upgrade pip setuptools build twine
python -m pip install --upgrade pip setuptools build twine openai
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do something where we add a version number to the matrix and pin that version number here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No i think we want the test to fail if openai releases another breaking change :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure I just think we could also make sure it's tested across all the versions we do support. Perhaps there's a way to encode "no version" as one of the options?


openai_obj = openai
is_v1 = False
if hasattr(openai, "chat") and hasattr(openai.chat, "completions"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we expecting that someone has already imported openai and set openai.api_key before getting here? Because if I just import openai from scratch and call `hasattr(openai.chat, "completions"), I see the following error:

     91     api_key = os.environ.get("OPENAI_API_KEY")
     92 if api_key is None:
---> 93     raise OpenAIError(
     94         "The api_key client option must be set either by passing api_key to the client or by setting the OPENAI_API_KEY environment variable"
     95     )
     96 self.api_key = api_key
     98 if organization is None:

OpenAIError: The api_key client option must be set either by passing api_key to the client or by setting the OPENAI_API_KEY environment variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah weird, I changed it to be hasattr(openai, "OpenAI").

@ankrgyl ankrgyl merged commit 7a24755 into main Nov 10, 2023
3 checks passed
ankrgyl added a commit to braintrustdata/braintrust-sdk that referenced this pull request Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants