Sd2 107 add new embeddings providers #219
Conversation
| @@ -63,7 +63,7 @@ def text__summarize( | |||
| def image__embeddings( | |||
| self, | |||
| file: str, | |||
| representation: str, | |||
| representation: Optional[str], | |||
There was a problem hiding this comment.
Maybe you should a default value if representation is not provided:
representation: Optional[str] = None,| emotion_output[ | ||
| emo.get("Type").lower() | ||
| ] = standardized_confidence_score(normalized_emo / 100) | ||
| emotion_output[emo.get("Type").lower()] = ( |
There was a problem hiding this comment.
Just a small details, but I would have write: emo["Type] instead of emo.get("Type") as we are sure it has a value because there is an IF above
| self, | ||
| file: str, | ||
| file_url: str = "", |
There was a problem hiding this comment.
I would have set file_url at the end, as it's the parameter the less likely to be set by a user if he uses our package. But it's also a small detail
|
|
||
| items: Sequence[EmbeddingDataClass] = [] | ||
| items.append(EmbeddingDataClass(embedding=original_response["embedding"])) |
There was a problem hiding this comment.
here, I would have done,
original_response.get("embedding", []) or []in case there is not the key embedding
| def image__embeddings( | ||
| self, | ||
| file: Optional[str], | ||
| file_url: Optional[str] = "", |
There was a problem hiding this comment.
same here, but small details
| items: Sequence[EmbeddingDataClass] = [] | ||
|
|
||
| for prediction in original_response["predictions"]: | ||
| items.append(EmbeddingDataClass(embedding=prediction["imageEmbedding"])) |
There was a problem hiding this comment.
same here also for the key "imageEmbedding"
| @@ -122,7 +122,7 @@ def image__anonymization( | |||
| def image__embeddings( | |||
| self, | |||
| file: str, | |||
| representation: str, | |||
| representation: Optional[str], | |||
There was a problem hiding this comment.
same here with a default value to None,
No description provided.