Multi queries #307
Merged
Multi queries #307
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pandu-k
commented
Feb 7, 2023
| @@ -45,7 +45,7 @@ def vectorise(model_name: str, content: Union[str, List[str]], model_properties: | |||
| try: | |||
| vectorised = available_models[model_cache_key].encode(content, normalize=normalize_embeddings, **kwargs) | |||
| except UnidentifiedImageError as e: | |||
| raise VectoriseError from e | |||
| raise VectoriseError(str(e)) from e | |||
There was a problem hiding this comment.
This adds the UnidentifiedImageError message to e
pandu-k
commented
Feb 7, 2023
| raise errors.InvalidArgError("Syntax error, could not parse filter string") from e | ||
| raise e | ||
| raise errors.BackendCommunicationError(f"Error communicating with Marqo-OS backend:\n{response}") |
There was a problem hiding this comment.
We pass through the Marqo-os error as fallback, if not yet handled
vicilliar
approved these changes
Feb 9, 2023
| @@ -6,7 +6,9 @@ | |||
| import os | |||
| import urllib | |||
| from tqdm import tqdm | |||
| from src.marqo.s2_inference.configs import ModelCache | |||
There was a problem hiding this comment.
mentioned this bug to li, i think he might have made the same change. looks good.
| ordered_queries = None | ||
|
|
||
| if isinstance(query, str): | ||
| to_be_vectorised = [query, ] |
There was a problem hiding this comment.
can this just be to_be_vectorised = [query]?
| ordered_queries = list(query.items()) | ||
| if index_info.index_settings[NsField.index_defaults][NsField.treat_urls_and_pointers_as_images]: | ||
| text_queries = [k for k, _ in ordered_queries if _is_image(k)] | ||
| image_queries = [k for k, _ in ordered_queries if not _is_image(k)] |
There was a problem hiding this comment.
could these be extracted in 1 loop to make it faster? like
for k, _ in ordered_queries:
if _is_image(k):
text_queries.append(k)
else:
image_queries.append(k)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
No way for weighted multi query searches
What is the new behavior (if this is a feature change)?
Weighted multi query searches.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No
Have unit tests been run against this PR? (Has there also been any additional testing?)
Yes
Related Python client/documentation etc changes (link commit/PR here)
Client: Multi query support py-marqo#54
API tests: API test for multi query marqo-api-tests#9
Documentation: https://gh-previews.marqo.pages.dev/0.0.13/API-Reference/search/#query-q
Please check if the PR fulfills these requirements