Skip to content

Conversation

@ohmayr
Copy link
Contributor

@ohmayr ohmayr commented Jan 16, 2026

Docs diff for vision api:

# Copyright 2018 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import re
from typing import Optional, Dict

import pypandoc  # type: ignore

from gapic.utils.lines import wrap

# Cache for the few complex items we actually send to pandoc
_RAW_RST_CACHE: Dict[str, str] = {}

def _tuned_fast_convert(text: str) -> Optional[str]:
    """
    Converts Markdown to RST using pure Python.
    Only falls back to Pandoc for Tables and Images.
    """
    # --- 1. FALLBACKS ---
    # Tables (pipe surrounded by spaces) or Images (![).
    # We allow "][" (Reference Links) to be handled by Python now.
    if (re.search(r" \| ", text) or re.search(r"\|\n", text)) or "![" in text:
        return None

    # --- 2. CONVERSION ---

    # A. CODE BLOCKS: `code` -> ``code``
    # CRITICAL: Run this FIRST. This ensures we handle existing backticks
    # before we create NEW backticks for links.
    # (?<!:) ensures we don't break Sphinx roles like :class:`MyClass`
    converted = re.sub(r"(?<!:|`)`([^`]+)`(?!`)", r"``\1``", text)
    
    # B. REFERENCE LINKS: [Text][Ref] -> `Text <Ref>`__
    # We fix the broken documentation by converting these to valid RST links.
    # Since step A is done, these new backticks will NOT be doubled.
    converted = re.sub(r"\[([^\]]+)\]\[([^\]]+)\]", r"`\1 <\2>`__", converted)

    # C. STANDARD LINKS: [Text](URL) -> `Text <URL>`__
    converted = re.sub(r"\[([^\]]+)\]\(([^)]+)\)", r"`\1 <\2>`__", converted)

    # D. BOLD/ITALICS:
    converted = re.sub(r"(?<!_)\b_([^_]+)_\b(?!_)", r"*\1*", converted)

    # E. HEADINGS: # Heading -> Heading\n=======
    converted = re.sub(r"^# (.*)$", r"\1\n" + "=" * 10, converted, flags=re.MULTILINE)
    converted = re.sub(r"^## (.*)$", r"\1\n" + "-" * 10, converted, flags=re.MULTILINE)

    # F. LISTS: Markdown (- item) needs a preceding newline for RST.
    converted = re.sub(r"(\n[^-*].*)\n\s*([-*] )", r"\1\n\n\2", converted)

    return converted

def rst(
    text: str,
    width: int = 72,
    indent: int = 0,
    nl: Optional[bool] = None,
    source_format: str = "commonmark",
):
    # 1. Super Fast Path: No special chars? Just wrap.
    if not re.search(r"[|*`_[\]#]", text):
        answer = wrap(text, indent=indent, offset=indent + 3, width=width - indent)
        return _finalize(answer, nl, indent)

    # 2. Check Cache
    if text in _RAW_RST_CACHE:
        raw_rst = _RAW_RST_CACHE[text]
    else:
        # 3. Try Tuned Python Convert (Fastest)
        fast_result = _tuned_fast_convert(text)
        
        if fast_result is not None:
            raw_rst = fast_result.strip()
        else:
            # 4. Fallback to Pandoc (Only for Tables/Images)
            raw_rst = pypandoc.convert_text(
                text,
                "rst",
                format=source_format,
                verify_format=False,
                extra_args=["--columns=%d" % (width - indent)],
            ).strip()
            
        _RAW_RST_CACHE[text] = raw_rst

    # 5. Python Formatting
    if "::" in raw_rst or ".. code" in raw_rst:
        answer = raw_rst.replace("\n", f"\n{' ' * indent}")
    else:
        answer = wrap(raw_rst, indent=indent, offset=indent, width=width - indent)

    return _finalize(answer, nl, indent)


def _finalize(answer, nl, indent):
    """Helper to handle trailing newlines and quotes."""
    if nl or ("\n" in answer and nl is None):
        answer += "\n" + " " * indent

    # If the text ends in a double-quote, append a period.
    # This ensures that we do not get a parse error when this output is
    # followed by triple-quotes.
    if answer.endswith('"'):
        answer += "."

    # Done; return the answer.
    return answer

Note: Client post processing scripts were disabled.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @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

  • Documentation Formatting Standardized: Docstrings across various Google Cloud Vision API client versions have been re-formatted to improve readability and ensure consistent RST rendering. This includes line wrapping and updating cross-references.
  • Client Structure Refinement: The VisionHelpers and add_single_feature_methods have been removed from __init__.py files in vision_v1 and v1p1beta1 modules, indicating a simplification or modernization of the client's internal structure.
  • Credential Handling Simplification: Credential loading logic in transport base files has been refactored to directly pass arguments instead of using an intermediate scopes_kwargs dictionary, streamlining the code.
  • Configuration Update: A new configuration file, .librarian/generate-request.json, has been added to define API versions, service configurations, and file handling rules for the Google Cloud Vision API.
  • Type Checking Version Update: The mypy configuration has been updated to target Python version 3.14, aligning with newer Python standards for type checking.

🧠 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 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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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 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

  1. 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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

The Python version 3.14 is invalid and will likely cause mypy to fail. Please correct this to the intended target Python version, for example, 3.12.

python_version = 3.12

Comment on lines 2652 to 2662
: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.
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This docstring contains several reStructuredText (RST) syntax errors that will likely break documentation rendering:

  1. There's an extra backtick at the end of the :class: role: ``:class:`google.cloud.vision_v1.types.ImportProductSetsResponse`````.
  2. The method name `ImportProductSets` is missing a closing backtick.
  3. The link text for google.longrunning.Operations.GetOperation and google.longrunning.Operation.response is 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.

Suggested change
: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.

Comment on lines 201 to 204
versioning is not supported. See `Google Cloud
Storage Request
URIs
<https://cloud.google.com/storage/docs/reference-uris>`__
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The link text Google Cloud Storage Request URIs is broken across multiple lines. This is invalid reStructuredText syntax and will cause rendering issues in the documentation. Please ensure link text remains on a single line. This issue appears in many places throughout the pull request.

Comment on lines +4 to +25
"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"
}
],
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For better readability and easier maintenance, consider sorting the API versions in the apis array. For example, v1, v1p1beta1, v1p2beta1, etc.

Comment on lines 744 to 747
* 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

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.

Suggested change
* 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.

Comment on lines 2832 to 2834
service Foo {
rpc Bar(google.protobuf.Empty)
returns (google.protobuf.Empty); }
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

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.

Suggested change
service Foo {
rpc Bar(google.protobuf.Empty)
returns (google.protobuf.Empty); }
service Foo {
rpc Bar(google.protobuf.Empty) returns (google.protobuf.Empty);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant