-
Notifications
You must be signed in to change notification settings - Fork 395
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
Adding AQA (GenerateAnswer). #169
Conversation
Change-Id: Ia95d8b2c48506f843f765c0546e848b46f6d8d32
google/generativeai/answer.py
Outdated
setattr(self, key, value) | ||
|
||
self.result = None | ||
if self.answer: |
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.
This should have the same quick access properties as GenerateContentResponse
> .parts
and .text
.
google/generativeai/answer.py
Outdated
client = get_default_generative_client() | ||
|
||
response = client.generate_answer(request) | ||
response = type(response).to_dict(response) |
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.
For the rest of the "GenerativeService" APIs I've just been using the protos as it. They're ugly when you print them, but we should stay in sync on that.
Hi Shilpa, Since you first opened this PR, a new field input_feedback has been added to GenerateAnswerResponse in v0.5.0. Do you think you could incorporate the usage of this new field into this PR? That might be simpler than sending a separate PR after this one is merged. If you take Mark's suggestion of using GenerateAnswerResponse instead of an Answer wrapper class, I think that would take care of it. Thanks or let me know if you don't have cycles to deal with my new request and I'll try to do it (I'm not familiar with this codebase or even github though) |
|
||
if safety_settings: | ||
safety_settings = safety_types.normalize_safety_settings( | ||
safety_settings, harm_category_set="new" |
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.
Note:
GenerateAnswer is currently using the "old" HarmCategory set, but it will switch over to "new" very shortly.
I think you should leave this as "new", because the switch from old to new should be complete by early next week, probably sooner than when this code gets released.
But just FYI you might see an exception "AQA does not yet support custom harassment safety thresholds" in the meantime.
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.
Thanks for the warning.
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.
We'll test against the API before making the release.
Co-authored-by: Mark Daoust <[email protected]>
|
||
if safety_settings: | ||
safety_settings = safety_types.normalize_safety_settings( | ||
safety_settings, harm_category_set="new" |
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.
Thanks for the warning.
|
||
if safety_settings: | ||
safety_settings = safety_types.normalize_safety_settings( | ||
safety_settings, harm_category_set="new" |
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.
We'll test against the API before making the release.
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.
Lets go 🛫.
Change-Id: Ia95d8b2c48506f843f765c0546e848b46f6d8d32
Description of the change
Adding AQA API (Python)