feat: add support for image embeddings #1753
Conversation
collindutter
force-pushed
the
feature/image-embeddings-vector
branch
from
08ccde7 to
79e7dcf
Compare
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
collindutter
force-pushed
the
feature/image-embeddings-vector
branch
6 times, most recently
from
4459f3d to
159a25c
Compare
collindutter
force-pushed
the
feature/image-embeddings-vector
branch
6 times, most recently
from
7d925d3 to
bc85088
Compare
collindutter
changed the title
collindutter
force-pushed
the
feature/image-embeddings-vector
branch
from
bc85088 to
bc5d697
Compare
| - [embed_string()](../../reference/griptape/drivers/embedding/base_embedding_driver.md#griptape.drivers.embedding.base_embedding_driver.BaseEmbeddingDriver.embed_string) for any string. | ||
|
|
||
| You can optionally provide a [Tokenizer](../misc/tokenizers.md) via the [tokenizer](../../reference/griptape/drivers/embedding/base_embedding_driver.md#griptape.drivers.embedding.base_embedding_driver.BaseEmbeddingDriver.tokenizer) field to have the Driver automatically chunk the input text to fit into the token limit. | ||
| Embeddings in Griptape are multidimensional representations of text and image data. Embeddings carry semantic information, which makes them useful for extracting relevant chunks from large bodies of text for search and querying. |
There was a problem hiding this comment.
would "text or image data" be better as they are embedding separately from each other?
In the last sentence, we should include "images" somehow so:
which makes them useful for extracting relevant information from large bodies of text or image data for search and querying.
| @@ -40,18 +36,80 @@ def to_artifact(self) -> BaseArtifact: | |||
|
|
|||
| def upsert_text_artifacts( | |||
| self, | |||
| artifacts: list[TextArtifact] | dict[str, list[TextArtifact]], | |||
| artifacts: list[TextArtifact] | dict[str, list[TextArtifact] | list[ImageArtifact]], | |||
There was a problem hiding this comment.
why do we want to support ImageArtifact in a method called upsert_text_artifacts? if we do keep it, should the first type also be list[TextArtifact] | list[ImageArtifact]
| @overload | ||
| def upsert_collection( | ||
| self, | ||
| artifacts: list[TextArtifact], |
There was a problem hiding this comment.
should we support both text and image here?
|
|
||
| def upsert_collection( | ||
| self, | ||
| artifacts: list[TextArtifact] | dict[str, list[TextArtifact] | list[ImageArtifact]], |
There was a problem hiding this comment.
same commont on first type, or list[ImageArtifact]
| self, | ||
| artifact: TextArtifact, | ||
| value: str | TextArtifact | ImageArtifact, |
There was a problem hiding this comment.
since we are allowing users to input str types, should we also allow bytes if a user has a loaded image already in memory they can upsert that may not be wrapped as an ImageArtifact?
| @@ -97,6 +98,8 @@ def query( | |||
|
|
|||
| Performs a query on the Knowledge Base and returns Artifacts with close vector proximity to the query, optionally filtering to only those that match the provided filter(s). | |||
| """ | |||
| if isinstance(query, BaseArtifact): | |||
There was a problem hiding this comment.
why allow the type, just to raise an error later? what if someone tries to pass a TextArtifact? should the type be ImageArtifact instead?
Describe your changes
Sorry for big-ish PR, lots of overlapping changes here:
embedmethod rather than type-specific methods (embed_string, embed_image, etc).a. Add support for embedding images in Amazon Bedrock Titan Embedding Driver.
b. Add support for embedding images in VoyageAi Embedding Driver.
upsertmethod rather than type-specific methods (upsert_text,upsert_text_artifact, etc).a. Add support to Local Vector Store Driver to save
Entrys with Image Artifacts into thepersist_file.Issue ticket number and link
Closes #1729