-
Notifications
You must be signed in to change notification settings - Fork 313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Semantic retriever #168
Semantic retriever #168
Conversation
Change-Id: I5c4f35238f3bc0bbc798abd72cf824ebe1103152
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the tests.
Co-authored-by: Mark Daoust <markdaoust@google.com>
Co-authored-by: Mark Daoust <markdaoust@google.com>
Co-authored-by: Mark Daoust <markdaoust@google.com>
Co-authored-by: Mark Daoust <markdaoust@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test like test_async_code_match
?
client = get_default_retriever_async_client() | ||
|
||
if results_count: | ||
if results_count < 0 or results_count >= 100: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here for the result counts
def delete_document( | ||
self, | ||
name: str, | ||
force: Optional[bool] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the user specifies it wouldn't we pass None to the request on line 444? Either we make the default false or if none, we don't set it in the request in line 444
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get this:
File "/opt/homebrew/lib/python3.11/site-packages/proto/message.py", line 558, in init
raise TypeError(
TypeError: Invalid constructor input for DeleteDocumentRequest: 'corpusname'
We'll resolve the open comments in follow on PRs before making the release. |
Change-Id: I5c4f35238f3bc0bbc798abd72cf824ebe1103152
Description of the change
Adding Semantic Retriever Python Client Library.