-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
community[minor]: add FirestoreVectorStore #5290
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
For a community contribution, I imagine what is also needed are
I'd welcome guidance or contribution on this, as well as the code itself. In the mean the code is working as-is when used in a local project of mine |
For my reference mainly, #655 seems a good PR to compare against what needs to be done |
Just to note, I'm using FirestoreVectorStore in production in my own project so I am maintaining the code in my local codebase. I'm going to leave this PR alone until there is other demand out there in the community. Ping if interest comes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a good start! There are a couple things this will need before I can merge though:
- documentation. See this doc on how to generate a template
- JSDocs. We've started adding JSDocs with examples to new integrations. See Chroma for an example
- Formatting/lint. I can see some issues which will cause our formatter to fail this, please run
yarn format && yarn lint:fix
fromlibs/langchain-community
.
Once these are in, please tag me for a review and I'll check it out again. Thank you!
* @param documents Documents to be added. | ||
* @returns Promise that resolves when the documents have been added. | ||
*/ | ||
async addDocuments(documents: Document[], options?: { ids?: string[] }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For document input arguments, the type should be DocumentInterface
, and for return types, Document
.
async addDocuments(documents: Document[], options?: { ids?: string[] }) { | |
async addDocuments(documents: DocumentInterface[], options?: { ids?: string[] }) { |
* @returns Promise that resolves to a new instance of FirestoreVectorSearch. | ||
*/ | ||
static async fromDocuments( | ||
docs: Document[], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs: Document[], | |
docs: DocumentInterface[], |
nit, same as above
*/ | ||
async addVectors( | ||
vectors: number[][], | ||
documents: Document[], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documents: Document[], | |
documents: DocumentInterface[], |
@glorat is attempting to deploy a commit to the LangChain Team on Vercel. A member of the Team first needs to authorize it. |
Implements #5289