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

community[minor]: Add indexing via locality sensitive hashing to the Yellowbrick vector store #20856

Merged
merged 81 commits into from May 6, 2024

Conversation

markcusack
Copy link
Contributor

  • Description: Add LSH-based indexing to the Yellowbrick vector store module
  • Twitter handle: @markcusack

markcusack and others added 30 commits November 24, 2023 11:30
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
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label May 2, 2024

if not isinstance(embedding, Embeddings):
warnings.warn("embeddings input must be Embeddings object.")
self.logger.error("embeddings input must be Embeddings object.")
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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,
Copy link
Collaborator

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:
Copy link
Collaborator

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

@eyurtsev eyurtsev changed the title Add indexing via locality sensitive hashing to the Yellowbrick vector store community[minor]: Add indexing via locality sensitive hashing to the Yellowbrick vector store May 6, 2024
@eyurtsev eyurtsev enabled auto-merge (squash) May 6, 2024 18:42
@eyurtsev
Copy link
Collaborator

eyurtsev commented May 6, 2024

Left a few comment -- feel free to address in a separate PR if you think they're important. Merging as soon as tests pass.

@eyurtsev eyurtsev merged commit 060987d into langchain-ai:master May 6, 2024
61 checks passed
dglogo pushed a commit to dglogo/langchain that referenced this pull request May 8, 2024
…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]>
kyle-cassidy pushed a commit to kyle-cassidy/langchain that referenced this pull request May 10, 2024
…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]>
kyle-cassidy pushed a commit to kyle-cassidy/langchain that referenced this pull request May 16, 2024
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases lgtm PR looks good. Use to confirm that a PR is ready for merging. size:XXL This PR changes 1000+ lines, ignoring generated files. Ɑ: vector store Related to vector store module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants