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

langchain_chroma: added document.id support #27995

Merged
merged 4 commits into from
Dec 4, 2024
Merged

langchain_chroma: added document.id support #27995

merged 4 commits into from
Dec 4, 2024

Conversation

epinzur
Copy link
Contributor

@epinzur epinzur commented Nov 8, 2024 •
edited
Loading

Description:

  • Added internal Document.id support to Chroma VectorStore

Dependencies:

Tests:

  • Modified/Added tests for Document.id support. All tests are passing.

Note: I am not a member of the Chroma team.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Nov 8, 2024
@vercel Vercel
Copy link

vercel bot commented Nov 8, 2024 •
edited
Loading

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

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2024 11:59pm

@dosubot dosubot bot added the Ɑ: vector store Related to vector store module label Nov 8, 2024
@efriis efriis self-assigned this Nov 20, 2024
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Dec 3, 2024
@efriis efriis enabled auto-merge (squash) December 3, 2024 23:59
@efriis efriis merged commit eff8a54 into langchain-ai:master Dec 4, 2024
19 checks passed
@ccurme ccurme mentioned this pull request Dec 5, 2024
Merged
ccurme added a commit that referenced this pull request Dec 5, 2024
@ccurme
chroma[patch]: fix bug (#28538)
8f9b3b7 
Fix bug introduced in
#27995

If all document IDs are `""`, the chroma SDK will raise
```
DuplicateIDError: Expected IDs to be unique
```

Caught by [docs
tests](https://github.com/langchain-ai/langchain/actions/runs/12180395579/job/33974633950),
but added a test to langchain-chroma as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bruno-ramirez-w bruno-ramirez-w bruno-ramirez-w approved these changes

@efriis efriis efriis approved these changes

Assignees

@efriis efriis

Labels
lgtm PR looks good. Use to confirm that a PR is ready for merging. size:L This PR changes 100-499 lines, ignoring generated files. Ɑ: vector store Related to vector store module
Projects
PR Reviews
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@epinzur @efriis @bruno-ramirez-w @eyurtsev