Copilot commented on code in PR #250:
URL:
https://github.com/apache/incubator-hugegraph-ai/pull/250#discussion_r2103998263
##########
hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py:
##########
@@ -63,8 +96,20 @@ def get_texts_embeddings(
A list of embedding vectors, where each vector is a list of floats.
The order of embeddings matches the order of input texts.
"""
- response = self.client.embed(model=self.model,
input=texts)["embeddings"]
- return [list(inner_sequence) for inner_sequence in response]
+ if hasattr(self.client, "embed"): # pylint: disable=no-else-return
+ response = self.client.embed(model=self.model,
input=texts)["embeddings"]
+ return [list(inner_sequence) for inner_sequence in response]
+ elif hasattr(self.client, "embeddings"):
+ embeddings_list = []
+ for text_item in texts:
+ response_item = self.client.embeddings(model=self.model,
prompt=text_item)
+ embeddings_list.append(list(response_item["embedding"]))
+ return embeddings_list
Review Comment:
[nitpick] When using the 'embeddings' method, the code makes individual API
calls for each text. Consider checking if a batch API is available or batching
these calls to reduce potential network overhead.
```suggestion
response = self.client.embeddings(model=self.model, prompt=texts)
try:
return [list(embedding) for embedding in
response["embeddings"]]
except KeyError as e:
raise RuntimeError(
"Failed to extract embeddings from Ollama client
'embeddings' response. "
f"Response: {response}. Error: {e}"
) from e
```
##########
hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py:
##########
@@ -39,8 +39,41 @@ def get_text_embedding(
self,
text: str
) -> List[float]:
- """Comment"""
- return list(self.client.embed(model=self.model,
input=text)["embeddings"][0])
+ """Get embedding for a single text.
+
+ This method handles different Ollama client API versions by checking
for
+ the presence of 'embed' or 'embeddings' methods.
+ """
+ if hasattr(self.client, "embed"):
+ response = self.client.embed(model=self.model, input=text)
+ try:
+ # First, try the structure typically seen for single embeddings
+ # or newer batch responses that might return a single
"embedding" key.
+ return list(response["embedding"])
+ except KeyError:
+ # Fallback for older batch-like response for single item,
+ # or if "embeddings" is a list with one item.
+ try:
+ return list(response["embeddings"][0])
+ except (KeyError, IndexError) as e:
+ raise RuntimeError(
+ "Failed to extract embedding from Ollama client
'embed' response. "
+ f"Response: {response}. Error: {e}"
+ ) from e
+ elif hasattr(self.client, "embeddings"):
+ response = self.client.embeddings(model=self.model, prompt=text)
+ try:
+ return list(response["embedding"])
+ except KeyError as e:
+ raise RuntimeError(
+ "Failed to extract embedding from Ollama client
'embeddings' response. "
+ f"Response: {response}. Error: {e}"
+ ) from e
Review Comment:
[nitpick] The nested try/except blocks could be refactored by first checking
the existence of keys in the response, which would improve clarity and reduce
reliance on exception handling.
```suggestion
# Check for the presence of "embedding" key in the response
if "embedding" in response:
return list(response["embedding"])
# Fallback for older batch-like response with "embeddings" key
elif "embeddings" in response and
isinstance(response["embeddings"], list) and len(response["embeddings"]) > 0:
return list(response["embeddings"][0])
else:
raise RuntimeError(
"Failed to extract embedding from Ollama client 'embed'
response. "
f"Response: {response}. Ensure the response structure is
correct."
)
elif hasattr(self.client, "embeddings"):
response = self.client.embeddings(model=self.model, prompt=text)
# Check for the presence of "embedding" key in the response
if "embedding" in response:
return list(response["embedding"])
else:
raise RuntimeError(
"Failed to extract embedding from Ollama client
'embeddings' response. "
f"Response: {response}. Ensure the response structure is
correct."
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]