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

partners: add similarity search by image functionality to langchain_chroma partner package #22982

Merged
merged 9 commits into from
Jul 15, 2024

Conversation

mrugank-wadekar
Copy link
Contributor

  • Description: This pull request introduces two new methods to the Langchain Chroma partner package that enable similarity search based on image embeddings. These methods enhance the package's functionality by allowing users to search for images similar to a given image URI. Also introduces a notebook to demonstrate it's use.
  • Issue: N/A
  • Dependencies: None
  • Twitter handle: @mrugank9009

@efriis efriis added the partner label Jun 17, 2024
@efriis efriis self-assigned this Jun 17, 2024
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 17, 2024
Copy link

vercel bot commented Jun 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 15, 2024 6:48pm

@dosubot dosubot bot added Ɑ: vector store Related to vector store module 🔌: chroma Primarily related to ChromaDB integrations 🤖:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features labels Jun 17, 2024
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.

way too many files have been changes, probably need to rebase or something

@mrugank-wadekar
Copy link
Contributor Author

Hi this is my first time contributing,
The first commit files were automatically changed, should I create a new PR so I will revert those auto changed files, and only push the actual code changes in it.?
Thanks!

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jun 18, 2024
…g CLIP and new features added to Chroma partner package from last commit .
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Jun 18, 2024
@mrugank-wadekar
Copy link
Contributor Author

I've modified my branch to affect only the .py files, do let me know if there's anything more I need to do.
Thanks!

@ccurme ccurme added community Related to langchain-community and removed community Related to langchain-community labels Jun 18, 2024
@ccurme
Copy link
Collaborator

ccurme commented Jun 19, 2024

@tazarov would you mind taking a look at this one? Thanks!

@mrugank-wadekar
Copy link
Contributor Author

Hi @tazarov , can i expect a merge with this one?

@mrugank-wadekar
Copy link
Contributor Author

Would updating the branch help for CI Success? If not, would appreciate some guidance on this. Thanks!

@efriis efriis assigned ccurme and unassigned efriis Jul 10, 2024
@efriis efriis self-assigned this Jul 15, 2024
Copy link
Collaborator

@ccurme ccurme left a comment

Choose a reason for hiding this comment

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

Thanks! For future reference, CI was held up by code formatting. Please see contribution guidelines here for how to resolve.

Left a minor comment and would welcome any follow-up PRs. If you can put together a concise ~10 line snippet for demonstration, would be worth adding it:

  • To docs (by editing notebook here;
  • As an integration test.

if self._embedding_function is None or not hasattr(
self._embedding_function, "embed_image"
):
raise ValueError("The embedding function must support image embedding.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider explicitly spelling out the required name here ("embed_image")

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jul 15, 2024
@ccurme ccurme enabled auto-merge (squash) July 15, 2024 18:42
@ccurme ccurme merged commit 66bebeb into langchain-ai:master Jul 15, 2024
36 checks passed
@mrugank-wadekar
Copy link
Contributor Author

mrugank-wadekar commented Jul 16, 2024

Hi thanks, for merging!
Learned a lot in my first PR into langchain.
I will add a small demonstration in the docs notebook for Chroma, in a separate PR.
A quick question, I have no idea what to do when I clone the fork into my local and some 62 or so files get auto modified?
This would be really helpful, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔌: chroma Primarily related to ChromaDB integrations 🤖:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features lgtm PR looks good. Use to confirm that a PR is ready for merging. partner size:L This PR changes 100-499 lines, ignoring generated files. Ɑ: vector store Related to vector store module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants