feat(translate): add region tag 'import_client_library' and refactor sample #13255
Conversation
There was a problem hiding this comment.
Hello @eapl-gemugami, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request addresses an internal issue (b/405384416) by adding a region tag, removing markdown files, and fixing sample styling. Specifically, it adds the 'translate_v3_import_client_library' region tag to the translate_v3_translate_text.py sample, removes the AUTHORING_GUIDE.md and CONTRIBUTING.md files, and improves the styling of the sample code.
Highlights
- Region Tag: The pull request introduces a new region tag, 'translate_v3_import_client_library', to clearly mark the section where the Google Cloud Translation library is imported.
- Markdown File Removal: Removes the AUTHORING_GUIDE.md and CONTRIBUTING.md files from the translate/samples directory, likely as part of a cleanup after a repository migration.
- Sample Styling: The styling of the translate_v3_translate_text.py sample has been improved, including adding a comment to initialize the Translation client and reformatting comments for better readability.
Changelog
- translate/samples/AUTHORING_GUIDE.md
- Removed the file.
- translate/samples/CONTRIBUTING.md
- Removed the file.
- translate/samples/snippets/translate_v3_translate_text.py
- Added region tag 'translate_v3_import_client_library' around the import statement for the Google Cloud Translation library (lines 18-21).
- Added a comment to initialize the Translation client (line 39).
- Reformatted comments for better readability, including adding a newline after the language code description (lines 35-36) and changing 'Supported mime types' to 'Supported MIME types' (line 44).
- Added a period to the end of the sentence 'Display the translation for each input text provided' (line 54).
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Trivia time!
What company developed the Google Cloud Translation API?
Click here for the answer
Google developed the Google Cloud Translation API.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request adds a region tag, removes Markdown files, and fixes the styling of the sample. The changes look good overall, with a few minor suggestions for improvement.
Summary of Findings
- Documentation Clarity: Clarify the purpose of initializing the Translation client with a comment before the initialization, to improve readability.
- Comment Consistency: Ensure consistency in commenting style, such as using periods at the end of comments for clarity.
Merge Readiness
The pull request is almost ready for merging. Addressing the minor suggestions regarding documentation clarity and comment consistency would further enhance the code quality. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging.
|
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
| @@ -31,13 +32,17 @@ def translate_text( | |||
| text: The content to translate. | |||
| language_code: The language code for the translation. | |||
There was a problem hiding this comment.
suggestion here: The ISO 639-1 language code of the target language (instead of "The language code for the translation")
There was a problem hiding this comment.
I agree with the suggestion, although documentation says:
Most language code parameters conform to ISO-639 identifiers, except where noted.
Leaving "The language code", and the link to the language models should be enough as there are many different variations and exceptions.
It's funny, the en-US used in source_language_code line 51 below, is not in the supported list, only en, although it works.
|
@iennae looking into your feedback I seized the opportunity to refactor the sample as David told me for previous cases. I think the original sample was inconsistent on its purpose of 'translating text', for example only translating from English to another language, so I added the concept of source and target languages as parameters, which is like usually the translating platforms work. I also removed the MIME type, not strictly required for this example, and cleaned it a bit the sample and the test. |
| https://cloud.google.com/translate/docs/languages#neural_machine_translation_model | ||
| """ | ||
|
|
||
| # Initialize Translation client. | ||
| client = translate_v3.TranslationServiceClient() | ||
| parent = f"projects/{PROJECT_ID}/locations/global" | ||
|
|
||
| # Translate text from English to chosen language. | ||
| # Supported MIME types: | ||
| # https://cloud.google.com/translate/docs/supported-formats |
There was a problem hiding this comment.
mime_type defaults to text/plain (source), but I think it's important to still highlight that this API supports different mime types.
There was a problem hiding this comment.
I did think the same, although that link says:
If left blank, the MIME type defaults to "text/html".
Perhaps that Doc is outdated related to the client library? I'll take a look into it
|
@iennae @glasnt Thanks for your review! My thought process for this refactor starts at this page (sample appears in other pages as well): https://cloud.google.com/translate/docs/advanced/translating-text-v3#translating_text Based on the title and context, I noticed the following:
That said, these are suggestions based on my first look at the sample. I'd need to check all the translate samples in context, or know the intent behind this sample. Please let me know if my suggestion would work, otherwise I can revert it. |
There was a problem hiding this comment.
Going by the source you linked, I've made some comments to address the additional changes made, while trying to keep in line with the sample style guide and similarity with other implementations (e.g. not all implementations take a source language in the function definition, but may use a default value).
|
@glasnt I have applied the feedback from your latest comment. I only have a question on this approach for the MIME type. If that looks correct, it's ready to be reviewed again. Thanks! |
|
cc @alarconesparza @OremGLG @hivanalejandro (Team colleagues) |
|
LGTM 👍 |
…ibility with '_v3beta1'
…ing compatibility with vertexai
…on 3.12 even when it was tested locally
Description
Fixes Internal:
b/405384416
Checklist
nox -s py-3.9(see Test Environment Setup)nox -s lint(see Test Environment Setup)