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.

Sign up for GitHub

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

Jump to bottom

Update qdrant interface #3971

Merged
merged 3 commits into from May 5, 2023
Merged

Update qdrant interface #3971

merged 3 commits into from May 5, 2023

Conversation

joein
Copy link
Contributor

@joein joein commented May 2, 2023

Hello

  1. Passing embedding_function as a callable seems to be outdated and the common interface is to pass Embeddings instance

  2. At the moment Qdrant.add_texts is designed to be used with embeddings.embed_query, which is 1) slow 2) causes ambiguity due to 1.
    It should be used with embeddings.embed_documents

This PR solves both problems and also provides some new tests

hwchase17
hwchase17 approved these changes May 4, 2023
View reviewed changes
Copy link
Contributor

@hwchase17 hwchase17 left a comment

Choose a reason for hiding this comment

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

lgtm! thanks

@hwchase17 hwchase17 added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label May 4, 2023
@joein
Copy link
Contributor Author

joein commented May 4, 2023

Hi, @hwchase17
Great, thanks
I’ve just fixed linter complaints, it should work now

@dev2049 dev2049 merged commit 2324f19 into langchain-ai:master May 5, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hwchase17 hwchase17 hwchase17 approved these changes

Assignees
No one assigned
Labels
lgtm PR looks good. Use to confirm that a PR is ready for merging.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@joein @hwchase17 @dev2049