-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(genai sdk): text generation samples (2nd batch) #5125
base: main
Are you sure you want to change the base?
Conversation
Here is the summary of changes. You are about to add 2 region tags.
This comment is generated by snippet-bot.
|
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.
Hi team,
I'm currently reviewing this pull request and will post my feedback in a few minutes. In the meantime, here's a summary of the changes for you and other reviewers to quickly get up to speed:
This pull request, authored by Valeriy-Burlaka, aims to add samples for text generation using the new GenAI SDK (second batch). The changes include:
- Go version update: The
go.mod
files across numerous directories have been updated fromgo 1.22.0
togo 1.23.0
. - New GenAI SDK samples: New code has been added for text generation, including:
text_generation/text_generation_test.go
: Unit tests for the text generation functionality. (Lines 1-54)text_generation/textgen_with_txt.go
: A sample demonstrating text generation using a text prompt. (Lines 1-61)text_generation/textgen_with_txt_img.go
: A sample demonstrating text generation using both text and image prompts. (Lines 1-68)
go.work
update: Thego.work
file has been updated to include the newgenai
directory. (Line 87)
The author has also completed a checklist ensuring tests pass, code is formatted, and relevant files (CODEOWNERS and blunderbuss) are updated. There is a request to merge the PR once approved.
I'll provide a more detailed review shortly.
Meanwhile, a little verse to lighten the mood:
Code flows like a river,
Tests run, a green light appears,
Bugs flee in the night.
Thanks,
Gemini
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 pull request adds samples for text generation using the GenAI SDK. The code is well-structured and includes appropriate tests. The samples demonstrate generating text with both text prompts and text/image prompts. The use of internal/testutil
for testing is correct. However, there are a few minor improvements that can be made, particularly around error handling and documentation, and updating the Go version in the go.mod files.
I've summarized the relevant elements from the contributing guidelines below:
- Print to an io.Writer: Snippets should print to an
io.Writer
passed as an argument. - Declare a context.Context: Declare a
context.Context
within the sample function. - Function arguments: Keep function arguments minimal.
- Return errors: Return errors with context using
fmt.Errorf
. - Testing: Tests are required and should use
testutil.SystemTest
. - Go Version: Use the minimum Go version supported by the repo.
return fmt.Errorf("unable to create genai client: %w", err) | ||
} | ||
|
||
resp, err := client.Models.GenerateContent(ctx, modelName, genai.Text("How does AI work?"), nil) |
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.
It's good practice to handle potential errors when calling GenerateContent
. Consider adding an error check and returning an appropriate error if the call fails.
resp, err := client.Models.GenerateContent(ctx, modelName, genai.Text("How does AI work?"), nil) | |
resp, err := client.Models.GenerateContent(ctx, modelName, genai.Text("How does AI work?"), nil) | |
if err != nil { | |
return fmt.Errorf("GenerateContent: %w", err) | |
} |
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.
Displays a wrong diff, the error is handled.
}}, | ||
}}, | ||
} | ||
resp, err := client.Models.GenerateContent(ctx, modelName, contents, nil) |
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.
Similar to the previous suggestion, it's good practice to handle potential errors when calling GenerateContent
. Consider adding an error check and returning an appropriate error if the call fails.
resp, err := client.Models.GenerateContent(ctx, modelName, contents, nil) | |
resp, err := client.Models.GenerateContent(ctx, modelName, contents, nil) | |
if err != nil { | |
return fmt.Errorf("GenerateContent: %w", err) | |
} |
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.
ditto
Description
Based on #5124, which should be merged first.
Adds samples for text generation using the new GenAI SDK (2nd batch). Added region tags:
googlegenaisdk_textgen_with_txt_img
googlegenaisdk_textgen_config_with_txt
googlegenaisdk_textgen_sys_instr_with_txt
googlegenaisdk_textgen_with_txt_stream
Checklist
go test -v ./..
(see Testing)gofmt
(see Formatting)go vet
(see Formatting)