-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Allow embeddings requests to be tokens or strings #417
Conversation
Codecov Report
@@ Coverage Diff @@
## master #417 +/- ##
==========================================
+ Coverage 96.95% 97.02% +0.06%
==========================================
Files 17 17
Lines 690 705 +15
==========================================
+ Hits 669 684 +15
Misses 15 15
Partials 6 6
|
embeddings.go
Outdated
} | ||
|
||
type BaseEmbeddingRequest struct { | ||
Input interface{} `json:"input"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you for the PR! Could you please change interface{}
to any
?
I would also like to make a suggestion.
|
embeddings.go
Outdated
// https://beta.openai.com/docs/api-reference/embeddings/create | ||
func (c *Client) CreateEmbeddings(ctx context.Context, request EmbeddingRequest) (resp EmbeddingResponse, err error) { | ||
req, err := c.newRequest(ctx, http.MethodPost, c.fullURL("/embeddings", request.Model.String()), withBody(request)) | ||
func (c *Client) CreateEmbeddings(ctx context.Context, body EmbeddingRequestBody) (resp EmbeddingResponse, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be clearer to specify the usable structs in the comments, now that the argument is an interface
Thanks for the feedback, everything mentioned should be included in my latest commits. Let me know if there's any other suggestions you have. |
Addresses #240
There may be a cleaner way to do this, but this is the best way I could think of quickly to not cause any breaking changes and to maintain some semblance of type checking for the user (instead of just changing the Input field type in EmbeddingRequest from string to interface{}).
Also modified the test case for it.
Thanks for all of the work you guys do on this project!