-
Notifications
You must be signed in to change notification settings - Fork 16.5k
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 indexing via locality sensitive hashing to the Yellowbrick vector store #20856
Conversation
markcusack
commented
Apr 24, 2024
- Description: Add LSH-based indexing to the Yellowbrick vector store module
- Twitter handle: @markcusack
A module that allows a Yellowbrick data warehouse instance to function as a vector store
Tests for Yellowbrick data warehouse
Added support for the Yellowbrick data warehouse
…d replacing with ON COMMIT DROP
|
||
if not isinstance(embedding, Embeddings): | ||
warnings.warn("embeddings input must be Embeddings object.") | ||
self.logger.error("embeddings input must be Embeddings object.") |
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.
Wy does this not raise an exception?
@@ -189,16 +408,110 @@ def from_texts( | |||
table: table to store embeddings | |||
kwargs: vectorstore specific parameters |
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.
new arguments are not documented
) | ||
cursor.execute(query) | ||
|
||
return None |
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.
nit: it's common to use an implicit return when the function is not meant to return anything (not important)
def __init__( | ||
self, | ||
embedding: Embeddings, | ||
connection_string: str, | ||
table: str, | ||
*, | ||
schema: Optional[str] = None, |
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.
undocumented parameters
|
||
def create_table_if_not_exists(self) -> None: |
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.
fyi this is a breaking change since it was a public method
Left a few comment -- feel free to address in a separate PR if you think they're important. Merging as soon as tests pass. |
…Yellowbrick vector store (langchain-ai#20856) - **Description:** Add LSH-based indexing to the Yellowbrick vector store module - **Twitter handle:** @markcusack --------- Co-authored-by: markcusack <[email protected]> Co-authored-by: markcusack <[email protected]> Co-authored-by: Eugene Yurtsev <[email protected]>
…Yellowbrick vector store (langchain-ai#20856) - **Description:** Add LSH-based indexing to the Yellowbrick vector store module - **Twitter handle:** @markcusack --------- Co-authored-by: markcusack <[email protected]> Co-authored-by: markcusack <[email protected]> Co-authored-by: Eugene Yurtsev <[email protected]>