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

partners[patch]: MongoDB vectorstore to return and accept string IDs #23818

Merged
merged 5 commits into from
Jul 3, 2024
Merged

partners[patch]: MongoDB vectorstore to return and accept string IDs #23818

merged 5 commits into from
Jul 3, 2024

Conversation

pprados
Copy link
Contributor

@pprados pprados commented Jul 3, 2024

@eyurtsev

The mongdb have some errors.

  • add_texts() -> List returns a list of ObjectId, and not a list of string
  • delete() with id never remove chunks.

@efriis efriis added the partner label Jul 3, 2024
@efriis efriis self-assigned this Jul 3, 2024
@vercel Vercel
Copy link

vercel bot commented Jul 3, 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 Jul 3, 2024 6:12pm

@pprados pprados marked this pull request as ready for review July 3, 2024 14:56
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. langchain Related to the langchain package Ɑ: vector store Related to vector store module 🔌: mongo Primarily related to Mongo integrations 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Jul 3, 2024
eyurtsev
eyurtsev approved these changes Jul 3, 2024
View reviewed changes
Copy link
Collaborator

@eyurtsev eyurtsev left a comment

Choose a reason for hiding this comment

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

Looks good, but let's separate into different PRs since the changes are very different conceptually.

FWIW I don't think we'll merge the change in format_instructions in the parser

libs/core/langchain_core/output_parsers/format_instructions.py Outdated Show resolved Hide resolved
libs/core/langchain_core/output_parsers/format_instructions.py Outdated Show resolved Hide resolved
libs/core/langchain_core/output_parsers/format_instructions.py Outdated Show resolved Hide resolved
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jul 3, 2024
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Jul 3, 2024
@eyurtsev eyurtsev changed the title partners[small]: Fix managment of id with mongdb: partners[patch]: Fix type of ID returned by add_texts in MongoDB vectorstore Jul 3, 2024
@eyurtsev eyurtsev changed the title partners[patch]: Fix type of ID returned by add_texts in MongoDB vectorstore partners[patch]: Cast ID returned by add_texts in MongoDB vectorstore to string Jul 3, 2024
@eyurtsev eyurtsev changed the title partners[patch]: Cast ID returned by add_texts in MongoDB vectorstore to string partners[patch]: MongoDB vectorstore to return and accept string IDs Jul 3, 2024
@eyurtsev eyurtsev merged commit 26cee2e into langchain-ai:master Jul 3, 2024
25 checks passed
@pprados pprados deleted the pprados/fix-mongodb-interface branch July 4, 2024 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eyurtsev eyurtsev eyurtsev approved these changes

Assignees

@efriis efriis

Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature langchain Related to the langchain package lgtm PR looks good. Use to confirm that a PR is ready for merging. 🔌: mongo Primarily related to Mongo integrations partner size:XS This PR changes 0-9 lines, ignoring generated files. Ɑ: vector store Related to vector store module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pprados @eyurtsev @efriis