Skip to content

Move YAPF style setting to a project-wide .style.yapf file#1038

Draft
mhucka wants to merge 2 commits intotensorflow:masterfrom
mhucka:add-yapf-config
Draft

Move YAPF style setting to a project-wide .style.yapf file#1038
mhucka wants to merge 2 commits intotensorflow:masterfrom
mhucka:add-yapf-config

Conversation

@mhucka
Copy link
Copy Markdown
Member

@mhucka mhucka commented Apr 10, 2026

Scripts and workflows that run yapf currently each add the option --style=google to the invocation of yapf. To follow best practices, this PR adds a .style.yapf configuration file at the top level of the project to set the style, and removes the flag from where it was added on the command line explicitly.

The use of a config file has the following benefits:

  • It lets tools like IDEs discover the setting automatically.
  • It reduces the chances that developers will forget to add the flag when they run yapf directly on the command line for quick checks, or write new scripts that use yapf.
  • It communicates intentions more clearly to new contributors.

Removing the flag from the scripts is not strictly necessary (everything will work the same if there is both a config file and the flag is passed to yapf). However, if a config file is used, it's better for maintainability and consistency to also remove the flag from individual invocations.

Set the YAPF style to Google style.
Copy link
Copy Markdown

@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 adds a .style.yapf configuration file to set the Python formatting style to Google. The review feedback suggests updating existing formatting scripts to remove hardcoded style flags, ensuring this new file serves as the single source of truth, and recommends adding a .clang-format file for C++ consistency.

@mhucka mhucka changed the title Add a .style.yapf Move YAPF style setting to a project-wide .style.yapf file Apr 10, 2026
Moved the style setting to the config file.
@mhucka
Copy link
Copy Markdown
Member Author

mhucka commented Apr 10, 2026

/gemini review

Copy link
Copy Markdown

@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 centralizes the YAPF configuration by introducing a .style.yapf file and removing redundant style flags from the formatting scripts. Feedback suggests invoking yapf via python3 -m in format_check.sh for better environment consistency and providing the filename parameter to the YAPF API in format_ipynb.py to ensure the new configuration file is properly detected.

if [[ "${changed_line_ranges}" != "--lines=0-0 " ]]; then
# Do the formatting.
results=$(yapf --style=google --diff "${changed_file}" ${changed_line_ranges})
results=$(yapf --diff "${changed_file}" ${changed_line_ranges})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For consistency with scripts/format_all.sh and to ensure the correct environment is used, it is recommended to invoke yapf using python3 -m yapf. This avoids potential issues where the yapf executable in the PATH might be associated with a different Python interpreter or version than the one intended for the project.

Suggested change
results=$(yapf --diff "${changed_file}" ${changed_line_ranges})
results=$(python3 -m yapf --diff "${changed_file}" ${changed_line_ranges})

try:
fmt_lines = yapf.yapf_api.FormatCode(''.join(lines),
style_config="google")[0]
fmt_lines = yapf.yapf_api.FormatCode(''.join(lines))[0]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

When using the yapf API without an explicit style_config, it is best practice to provide the filename argument. This allows yapf to search for the nearest configuration file (like the newly added .style.yapf) relative to the file being formatted, making the script more robust if run from different directories or if different parts of the project eventually require different styles.

Suggested change
fmt_lines = yapf.yapf_api.FormatCode(''.join(lines))[0]
fmt_lines = yapf.yapf_api.FormatCode(''.join(lines), filename=fname)[0]

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