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

feat(vertexai): pdf input #4112

Merged
merged 3 commits into from
May 1, 2024

Conversation

Deleplace
Copy link
Contributor

Go version of the Python sample at https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/generative_ai/gemini_pdf_example.py

Region tag generativeaionvertexai_gemini_pdf

@Deleplace Deleplace requested a review from a team as a code owner April 25, 2024 10:32
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Apr 25, 2024
Copy link

snippet-bot bot commented Apr 25, 2024

Here is the summary of changes.

You are about to add 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@Deleplace Deleplace enabled auto-merge (squash) April 25, 2024 13:02

res, err := model.GenerateContent(ctx, part, genai.Text(prompt))
if err != nil {
return fmt.Errorf("unable to generate contents: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

blocking: This and other fmt.Errorf instances, please wrap errors with %w.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome! I wasn't very familiar with %w, now I see it makes perfect sense

// generateContentFromPDF generates a response into w, based upon the prompt
// and the pdf file provided.
// pdf is a Google Cloud Storage path starting with "gs://"
func generateContentFromPDF(w io.Writer, prompt, pdf, projectID, location, modelName string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Follow the decision made as part of #4110 (review)

Comment on lines 28 to 30
// generateContentFromPDF generates a response into w, based upon the prompt
// and the pdf file provided.
// pdf is a Google Cloud Storage path starting with "gs://"
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Similar to #4113 (comment), suggest making the purpose of the code vs. what it does a little more clear.

Secondarily, my preference is to make the top-line comment independent of reading the function signature, so I would use "provided io.Writer" over specifying the variable name w.

Comment on lines 28 to 30
buf := new(bytes.Buffer)
prompt := `
Your are a very professional document summarization specialist.
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion:

Suggested change
buf := new(bytes.Buffer)
prompt := `
Your are a very professional document summarization specialist.
buf := new(bytes.Buffer)
prompt := `
You are a very professional document summarization specialist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thank you

Copy link
Contributor Author

@Deleplace Deleplace Apr 26, 2024

Choose a reason for hiding this comment

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

Also fixing upstream in Python: GoogleCloudPlatform/python-docs-samples#11592

@Deleplace Deleplace requested a review from grayside April 26, 2024 09:42
@Deleplace
Copy link
Contributor Author

Deleplace commented Apr 26, 2024

kokoro failing with Test_functionCallsChat: rpc error: .... but unrelated to the current code change

@Deleplace Deleplace merged commit e2c4add into GoogleCloudPlatform:main May 1, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants