Implement query_vector() for all vector_store_drivers#1494
Implement query_vector() for all vector_store_drivers#1494collindutter merged 9 commits intomainfrom
Conversation
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@cjkindel can you please create an issue with the original context of why this PR is being made? |
| def query( | ||
| self, | ||
| query: str, | ||
| *, | ||
| count: Optional[int] = None, | ||
| namespace: Optional[str] = None, | ||
| include_vectors: bool = False, | ||
| **kwargs, | ||
| ) -> list[BaseVectorStoreDriver.Entry]: | ||
| vector = self.embedding_driver.embed_string(query) | ||
| return self.query_vector(vector, count=count, namespace=namespace, include_vectors=include_vectors, **kwargs) | ||
|
|
There was a problem hiding this comment.
Can we move this implementation up to the base class since it appears to be common across most implementations?
There was a problem hiding this comment.
Done, left the classes that implement additional kwargs so that documentation will be complete.
| @abstractmethod | ||
| def query_vector( | ||
| self, | ||
| vector: list[float], | ||
| *, | ||
| count: Optional[int] = None, | ||
| namespace: Optional[str] = None, | ||
| include_vectors: bool = False, | ||
| **kwargs, | ||
| ) -> list[Entry]: ... | ||
|
|
There was a problem hiding this comment.
Adding abstract method is technically a breaking change for any users who have created their own vector store driver.
What do you think about:
| @abstractmethod | |
| def query_vector( | |
| self, | |
| vector: list[float], | |
| *, | |
| count: Optional[int] = None, | |
| namespace: Optional[str] = None, | |
| include_vectors: bool = False, | |
| **kwargs, | |
| ) -> list[Entry]: ... | |
| def query_vector( | |
| self, | |
| vector: list[float], | |
| *, | |
| count: Optional[int] = None, | |
| namespace: Optional[str] = None, | |
| include_vectors: bool = False, | |
| **kwargs, | |
| ) -> list[Entry]: | |
| # TODO: Mark as abstract method for griptape 2.0 | |
| raise NotImplementedError(f"{self.__class__.__name__} does not support vector query.") |
| !!! tip | ||
| Set `stream=True` on your [Prompt Driver](../drivers/prompt-drivers.md) in order to receive completion chunk events. | ||
|
|
||
| Set `stream=True` on your [Prompt Driver](../drivers/prompt-drivers.md) in order to receive completion chunk events. |
There was a problem hiding this comment.
I made sure I had latest formatter with make install and it still made this change so I think it is valid
There was a problem hiding this comment.
I've been battling this guy for weeks now. Let me try to fix on main before we merge this.
There was a problem hiding this comment.
Ah, can you add two tabs before Set? That should satisfy the formatter.
|
|
||
| Parameters: | ||
| query (str): Query string. | ||
| query (list[float]): Query vector. |
There was a problem hiding this comment.
Docs failing due to missing vector in docstring.
collindutter
left a comment
collindutter
left a comment
There was a problem hiding this comment.
Looks good, can you please add a CHANGELOG entry before you merge?
2 participants
Describe your changes
Add a query_vector() function to all vector_store_driver classes. This facilitates querying with existing vectors instead of generating vectors at query time.
Issue ticket number and link
Closes #1498