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

Fix for SVM retriever discarding document metadata #9141

Merged
merged 6 commits into from Aug 11, 2023

Conversation

MarkusSchiffer
Copy link
Contributor

As stated in the title the SVM retriever discarded the metadata of passed in docs. This code fixes that. I also added one unit test that should test that.

Ultimately it's a simple fix, but unfortunately poetry was not correctly installing dependencies in my environment so I was unable to run the unit test and linting (took way longer to try to set up environment than make my change). Would appreciate if someone with the environment set up could quickly try it out. I did test by directly modifying the langchain source code in my miniconda environment where I initially found the issue and it fixed the bug there, so it's not completely untested.

@vercel
Copy link

vercel bot commented Aug 11, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Aug 11, 2023 7:41pm

@dosubot dosubot bot added Ɑ: doc loader Related to document loader module (not documentation) 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Aug 11, 2023
@MarkusSchiffer MarkusSchiffer marked this pull request as draft August 11, 2023 19:02
@MarkusSchiffer MarkusSchiffer marked this pull request as ready for review August 11, 2023 19:03
@vercel vercel bot temporarily deployed to Preview – langchain August 11, 2023 19:17 Inactive
@baskaryan baskaryan added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Aug 11, 2023
@baskaryan
Copy link
Collaborator

thanks @MarkusSchiffer!

@MarkusSchiffer
Copy link
Contributor Author

Oh nice catch, I actually just thought of that edge case and you already made the change, phew!

@baskaryan baskaryan merged commit 00bf472 into langchain-ai:master Aug 11, 2023
22 checks passed
danielchalef pushed a commit to danielchalef/langchain that referenced this pull request Aug 11, 2023
As stated in the title the SVM retriever discarded the metadata of
passed in docs. This code fixes that. I also added one unit test that
should test that.
---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature Ɑ: doc loader Related to document loader module (not documentation) lgtm PR looks good. Use to confirm that a PR is ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants