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

Chroma vector store #1198

Merged
merged 4 commits into from
Sep 23, 2023
Merged

Chroma vector store #1198

merged 4 commits into from
Sep 23, 2023

Conversation

Chitti-Ankith
Copy link
Contributor

Added support for ChromaDB in Eva. Will focus on benchmarking the existing vector stores after this.

@xzdandy xzdandy added Integrations 🧩 Pull requests that update an integration Data Sources Features/Bugs related to Data Sources labels Sep 22, 2023
@xzdandy xzdandy added this to the v0.3.7 milestone Sep 22, 2023
@xzdandy
Copy link
Collaborator

xzdandy commented Sep 22, 2023

Hi @Chitti-Ankith , thanks for adding Chroma vector store support. Is this PR ready for review?

@Chitti-Ankith
Copy link
Contributor Author

Hi @xzdandy yes it is, can you please have a look?

xzdandy
xzdandy reviewed Sep 23, 2023
View reviewed changes
Copy link
Collaborator

@xzdandy xzdandy left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. Request @jiashenC 's review who has more expertise. We need to add a documentation page to summarize all vector database we supported and their available configuration parameters.

@Chitti-Ankith
Copy link
Contributor Author

Overall looks good to me. Request @jiashenC 's review who has more expertise. We need to add a documentation page to summarize all vector database we supported and their available configuration parameters.

I'll add the documentation once the benchmark is done

jiashenC
jiashenC requested changes Sep 23, 2023
View reviewed changes
Copy link
Member

@jiashenC jiashenC left a comment

Choose a reason for hiding this comment

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

It looks good overall. You might miss a test case in test_similarity

@Chitti-Ankith
Copy link
Contributor Author

It looks good overall. You might miss a test case in test_similarity

I added the chromadb requirement in circleci, can you tell me which test case will be missed?

@jiashenC
Copy link
Member

It looks good overall. You might miss a test case in test_similarity

I added the chromadb requirement in circleci, can you tell me which test case will be missed?

NVM, I misread your test cases. I think it is good to merge.

@Chitti-Ankith Chitti-Ankith merged commit e09cba9 into georgia-tech-db:staging Sep 23, 2023
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xzdandy xzdandy xzdandy left review comments

@jiashenC jiashenC jiashenC approved these changes

Assignees

@Chitti-Ankith Chitti-Ankith

Labels
Data Sources Features/Bugs related to Data Sources Integrations 🧩 Pull requests that update an integration
Projects
EVA Public Roadmap ⚡🚀
Archived in project
Milestone
v0.3.7
Development

Successfully merging this pull request may close these issues.
3 participants
@Chitti-Ankith @xzdandy @jiashenC