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

[BREAKING_CHANGES] convert EmbeddingModel to string type #629

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

jaffee
Copy link
Contributor

@jaffee jaffee commented Jan 3, 2024

This gives the user the ability to pass in models for embeddings that are not already defined in the library. Also more closely matches how the completions API works.

Describe the change
Addresses the issue described in #602 essentially that forcing embedding model to be represented as an int is awkward.

Provide OpenAI documentation link
The integer representation is never used by openai, only the string https://platform.openai.com/docs/api-reference/embeddings

Describe your solution
Gets rid of the extra layer of indirection and uses the string value directly.

Tests
This mostly ended up deleting code, and so deleted the associated test as well.

Additional context
Add any other context or screenshots or logs about your pull request here. If the pull request relates to an open issue, please link to it.

Issue: #602

This gives the user the ability to pass in models for embeddings that are not
already defined in the library. Also more closely matches how the completions
API works.
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c9615e0) 98.44% compared to head (40334f8) 98.43%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #629      +/-   ##
==========================================
- Coverage   98.44%   98.43%   -0.02%     
==========================================
  Files          24       24              
  Lines        1354     1342      -12     
==========================================
- Hits         1333     1321      -12     
  Misses         15       15              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sashabaranov
Copy link
Owner

@jaffee thank you so much for the PR! (and sorry for taking so long to review)

@sashabaranov sashabaranov merged commit e01a2d7 into sashabaranov:master Jan 15, 2024
3 checks passed
@jaffee jaffee deleted the embedding-model-string branch January 15, 2024 13:48
@jaffee
Copy link
Contributor Author

jaffee commented Jan 15, 2024

thanks @sashabaranov 🙏 . Lovely library, appreciate you.

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