Skip to content
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

Add genai.protos #354

Merged
merged 18 commits into from
May 30, 2024
Merged

Add genai.protos #354

merged 18 commits into from
May 30, 2024

Conversation

MarkDaoust
Copy link
Collaborator

@MarkDaoust MarkDaoust commented May 21, 2024

The main changes are:

  • Add genai.protos module, just containing from google.ai.generativelanguage_v1beta.types import *.
  • Replace all proto usage:
    • all import google.ai.generativelanguage as glm with from google.generativeai import protos.
    • all glm. with protos..
  • Keep client usage as-isglm.*Client.
  • Add a test to police glm.
  • Fix build_docs.py script.

Manual changes:

  • docs/build_docs.py
  • google/generativeai/protos.py
  • tests/test_protos.py

Change-Id: I21cfada033c6ffbed7a20e117e61582fde925f61
Change-Id: I9c8473d4ca1a0e92489f145a18ef1abd29af22b3
@github-actions github-actions bot added status:awaiting review PR awaiting review from a maintainer component:python sdk Issue/PR related to Python SDK labels May 21, 2024
@MarkDaoust MarkDaoust marked this pull request as draft May 21, 2024 16:46
MarkDaoust added 11 commits May 21, 2024 11:06
Change-Id: I576080fb80cf9dc9345d8bb2178eb4b9ac59ce97
Change-Id: I5f9aa3f8e3ae780e5cec2078d3eb153157b195fe
Change-Id: I5f7686ea121f16000acfa4d6e15a92d150c35344
Change-Id: I17014791d966d797b481bca17df69558b23a9a1a
Change-Id: I51d30f6568640456bcf28db2bd338a58a82346de
Change-Id: I4899231706c9624a0f189b22b6f70aeeb4cbea29
Change-Id: I297a5a6cc05607e806ed82cd4dc8e16358dfab54
Change-Id: I8a636fb634fd079a892cb99170a12c0613887ccf
Change-Id: I517171389801ef249cd478f98798181da83bef69
Change-Id: I8921c0caaa9b902ebde682ead31a2444298c2c9c
Change-Id: I1f6b3b9b9521baa8812a908431bf58c623860733
@MarkDaoust MarkDaoust marked this pull request as ready for review May 24, 2024 15:44
google/generativeai/protos.py Outdated Show resolved Hide resolved
google/generativeai/protos.py Outdated Show resolved Hide resolved
google/generativeai/protos.py Outdated Show resolved Hide resolved
tests/test_protos.py Outdated Show resolved Hide resolved
tests/test_protos.py Show resolved Hide resolved
google/generativeai/protos.py Show resolved Hide resolved
Change-Id: I0421a35687ed14b1a5ca3b496cafd91514c4de92
Change-Id: Ifc791796e36668eb473fd0fffea4833b1a062188
markmcd
markmcd previously approved these changes May 28, 2024
Copy link
Member

@markmcd markmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's quite a lot of content that ends up in the protos/ dir - e.g. protos.GenerateContentRequest - that has no use in this SDK, so might be confusing.

The case I'm thinking of is a user typing some variation of generate content into the filter on the API page, and landing on protos.GenerateContentRequest/Response.

It also looks like the generated docs for protos.* still refer to google.ai.generativelanguage - I think that's fine, since it's both the truth and a dependency - but it means fields like ToolConfig are no longer linked.

LGTM anyway as this is an improvement, and maybe doesn't matter with API ref changes coming.

tests/test_protos.py Show resolved Hide resolved
tests/test_protos.py Outdated Show resolved Hide resolved
google/generativeai/protos.py Outdated Show resolved Hide resolved
google/generativeai/protos.py Outdated Show resolved Hide resolved
Change-Id: Ieb900190f42e883337028ae25da3be819507db4a
Change-Id: I805473f9aaeb04e922a9f66bb5f40716d42fb738
@MarkDaoust
Copy link
Collaborator Author

protos.GenerateContentRequest - that has no use in this SDK, so might be confusing

Yes, it just didn't seem worth sorting through these 1 by one to decide which to include and which not to include.

still refer to google.ai.generativelanguage - I think that's fine, since it's both the truth and a dependency - but it means fields like ToolConfig are no longer linked.

Is there any way we can fix this? Add a find and replace in the generation process?

and maybe doesn't matter with API ref changes coming.

Yeah, that turns it into a real API-reference. But leaves us with nowhere that the SDKs are described in detail?

@markmcd markmcd merged commit f08c789 into google-gemini:main May 30, 2024
7 checks passed
@github-actions github-actions bot removed the status:awaiting review PR awaiting review from a maintainer label May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:python sdk Issue/PR related to Python SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants