Skip to content

feat(mypy): centralize mypy.ini and update templates#17523

Draft
chalmerlowe wants to merge 1 commit into
mainfrom
feat/centralize-mypy-v3
Draft

feat(mypy): centralize mypy.ini and update templates#17523
chalmerlowe wants to merge 1 commit into
mainfrom
feat/centralize-mypy-v3

Conversation

@chalmerlowe

Copy link
Copy Markdown
Contributor

Note

This is step one of a multi-step process. The work done here is outlined below.
Additional steps (to be completed in other PRs) include:

  • generate the generated packages
  • generate and/or post process hybrid packages

This work:

  • Adds a centralized mypy.ini file at the root of the repository.
  • Updates GAPIC generator templates to omit local mypy.ini and dynamically resolve the root config via a MYPY_CONFIG_FILE constant.

Note

Work on strictly handwritten libraries is outside the scope of this PR and can be found here: #17409

Partially resolves: #17322 🦕

> [!note]
> This is step one of a multi-step process. The work done here is outlined below.
Additional steps (to be completed in other PRs) include:
> * generate the **generated packages**
> * generate and/or post process **hybrid packages**

This work:
* Adds a centralized `mypy.ini` file at the root of the repository.
* Updates GAPIC generator templates to omit local `mypy.ini` and dynamically resolve the root config via a `MYPY_CONFIG_FILE` constant.

> [!note]
> Work on strictly handwritten libraries is outside the scope of this PR and can be found here: #17409

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 centralizes the mypy configuration by introducing a single mypy.ini at the repository root, removing individual package-level configurations, and updating the template and generated noxfile.py files to reference the centralized configuration. It also fixes a bug in the golden update script within integration_test.bzl. The reviewer feedback points out that the hardcoded relative path to mypy.ini will fail for integration test goldens, which are located deeper in the directory structure, and suggests dynamically searching upwards through parent directories to locate the configuration file.

Comment on lines +43 to +44
# Path to the centralized mypy configuration file at the repository root.
MYPY_CONFIG_FILE = str(CURRENT_DIRECTORY.parent.parent / "mypy.ini")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The hardcoded relative path CURRENT_DIRECTORY.parent.parent / 'mypy.ini' assumes that the generated package is always located exactly two levels down from the repository root. While this is true for standard packages in the monorepo, it is not true for integration test goldens (which are located five levels down at packages/gapic-generator/tests/integration/goldens/<package>). As a result, running nox -s mypy inside the golden directories will fail to find the centralized mypy.ini file.

To make this robust across all directory structures (including local development and integration tests), search upwards through parent directories to dynamically locate the mypy.ini file, falling back to the default relative path if not found.

# Path to the centralized mypy configuration file at the repository root.
# Search upwards to support running nox from both monorepo packages and integration test goldens.
MYPY_CONFIG_FILE = next(
    (str(p / "mypy.ini") for p in CURRENT_DIRECTORY.parents if (p / "mypy.ini").exists()),
    str(CURRENT_DIRECTORY.parent.parent / "mypy.ini"),
)
References
  1. Do not suggest manual code modifications, optimizations, or style fixes for generated files, as any changes should be implemented in the generator or templates to prevent them from being overwritten.

Comment on lines +10 to +11
CURRENT_DIRECTORY = pathlib.Path(__file__).parent.absolute()
MYPY_CONFIG_FILE = str(CURRENT_DIRECTORY.parent.parent / "mypy.ini")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The hardcoded relative path CURRENT_DIRECTORY.parent.parent / 'mypy.ini' assumes that the generated package is always located exactly two levels down from the repository root. While this is true for standard packages in the monorepo, it is not true for integration test goldens (which are located five levels down at packages/gapic-generator/tests/integration/goldens/<package>). As a result, running nox -s mypy inside the golden directories will fail to find the centralized mypy.ini file.

To make this robust across all directory structures (including local development and integration tests), search upwards through parent directories to dynamically locate the mypy.ini file, falling back to the default relative path if not found.

CURRENT_DIRECTORY = pathlib.Path(__file__).parent.absolute()
# Search upwards to support running nox from both monorepo packages and integration test goldens.
MYPY_CONFIG_FILE = next(
    (str(p / "mypy.ini") for p in CURRENT_DIRECTORY.parents if (p / "mypy.ini").exists()),
    str(CURRENT_DIRECTORY.parent.parent / "mypy.ini"),
)
References
  1. Do not suggest manual code modifications, optimizations, or style fixes for generated files, as any changes should be implemented in the generator or templates to prevent them from being overwritten.

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.

Move mypy.ini to the monorepo root.

1 participant