-
Notifications
You must be signed in to change notification settings - Fork 1.6k
experiment: docs diff for vision api #15467
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @ohmayr, 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! This pull request primarily focuses on enhancing the maintainability and consistency of the Google Cloud Vision API client libraries. It introduces standardized docstring formatting across multiple API versions, refactors internal client helper mechanisms, and updates the type checking configuration. These changes aim to improve code clarity and align with modern Python development practices. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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. Footnotes
|
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.
Code Review
This pull request appears to be an experiment in reformatting documentation strings across the Vision API client libraries, likely using an automated script. While the intent to improve documentation is good, and the associated code refactorings (like removing VisionHelpers and making credential handling more explicit) are positive changes, the script has introduced numerous formatting errors into the docstrings. These include invalid reStructuredText (RST) syntax such as broken links, incorrect backtick usage, and malformed code examples, which will likely break the documentation rendering. Additionally, there is a critical error in mypy.ini where the Python version is set to an invalid value. I've provided specific comments on representative examples of these issues.
| @@ -1,3 +1,3 @@ | |||
| [mypy] | |||
| python_version = 3.7 | |||
| python_version = 3.14 | |||
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.
| :class:`google.cloud.vision_v1.types.ImportProductSetsResponse`` | ||
| Response message for the | ||
| ``ImportProductSets` method. | ||
| This message is returned by the | ||
| `google.longrunning.Operations.GetOperation | ||
| <google.longrunning.Operations.GetOperation>`__ | ||
| method in the returned | ||
| `google.longrunning.Operation.response | ||
| <google.longrunning.Operation.response>`__ | ||
| field. |
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 docstring contains several reStructuredText (RST) syntax errors that will likely break documentation rendering:
- There's an extra backtick at the end of the
:class:role: ``:class:`google.cloud.vision_v1.types.ImportProductSetsResponse`````. - The method name
`ImportProductSets`is missing a closing backtick. - The link text for
google.longrunning.Operations.GetOperationandgoogle.longrunning.Operation.responseis broken across multiple lines, which is invalid syntax.
Please correct these formatting issues. This seems to be a recurring problem throughout the PR, likely caused by the generation script.
| :class:`google.cloud.vision_v1.types.ImportProductSetsResponse`` | |
| Response message for the | |
| ``ImportProductSets` method. | |
| This message is returned by the | |
| `google.longrunning.Operations.GetOperation | |
| <google.longrunning.Operations.GetOperation>`__ | |
| method in the returned | |
| `google.longrunning.Operation.response | |
| <google.longrunning.Operation.response>`__ | |
| field. | |
| :class:`google.cloud.vision_v1.types.ImportProductSetsResponse` | |
| Response message for the | |
| ``ImportProductSets`` method. | |
| This message is returned by the | |
| `google.longrunning.Operations.GetOperation <google.longrunning.Operations.GetOperation>`__ | |
| method in the returned | |
| `google.longrunning.Operation.response <google.longrunning.Operation.response>`__ | |
| field. |
| versioning is not supported. See `Google Cloud | ||
| Storage Request | ||
| URIs | ||
| <https://cloud.google.com/storage/docs/reference-uris>`__ |
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.
| "apis": [ | ||
| { | ||
| "path": "google/cloud/vision/v1p3beta1", | ||
| "service_config": "vision_v1p3beta1.yaml" | ||
| }, | ||
| { | ||
| "path": "google/cloud/vision/v1", | ||
| "service_config": "vision_v1.yaml" | ||
| }, | ||
| { | ||
| "path": "google/cloud/vision/v1p1beta1", | ||
| "service_config": "vision_v1p1beta1.yaml" | ||
| }, | ||
| { | ||
| "path": "google/cloud/vision/v1p2beta1", | ||
| "service_config": "vision_v1p2beta1.yaml" | ||
| }, | ||
| { | ||
| "path": "google/cloud/vision/v1p4beta1", | ||
| "service_config": "vision_v1p4beta1.yaml" | ||
| } | ||
| ], |
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.
| * Returns NOT_FOUND if the ProductSet does not exist. * | ||
| Returns INVALID_ARGUMENT if display_name is present in | ||
| update_mask but missing from the request or longer | ||
| than 4096 characters. |
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.
The "Possible errors" section incorrectly merges two distinct error conditions into a single bullet point. For clarity, each error condition should be its own bullet point.
| * Returns NOT_FOUND if the ProductSet does not exist. * | |
| Returns INVALID_ARGUMENT if display_name is present in | |
| update_mask but missing from the request or longer | |
| than 4096 characters. | |
| * Returns NOT_FOUND if the ProductSet does not exist. | |
| * Returns INVALID_ARGUMENT if display_name is present in | |
| update_mask but missing from the request or longer | |
| than 4096 characters. |
| service Foo { | ||
| rpc Bar(google.protobuf.Empty) | ||
| returns (google.protobuf.Empty); } |
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.
The formatting of this code example is broken. The returns line has incorrect indentation, and the closing brace } has trailing whitespace and is on the wrong line. This makes the example hard to read and incorrect.
| service Foo { | |
| rpc Bar(google.protobuf.Empty) | |
| returns (google.protobuf.Empty); } | |
| service Foo { | |
| rpc Bar(google.protobuf.Empty) returns (google.protobuf.Empty); | |
| } |
Docs diff for vision api:
Note: Client post processing scripts were disabled.