From 3e61db27c926f47943c9ea989cf88cc6f77401aa Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Tue, 28 Nov 2023 21:21:41 +0000 Subject: [PATCH] yapf: use yapf instead of autopep8 autopep8 does not support Python 3.12, which is a requirement for moving to fedora:39. The following ticket has been open without comment from the development community for 6 weeks: https://github.com/hhatto/autopep8/issues/712 Transitioning to yapf is easy, and the community behind it seems stronger. I think moving forward it is a better Python formatter for us. --- .style.yapf | 421 +++++++++++++++++++++++++++++++++++++++++++ .yapfignore | 18 ++ CMakeLists.txt | 13 +- tools/git/pre-commit | 37 ++-- tools/yapf.sh | 109 +++++++++++ 5 files changed, 578 insertions(+), 20 deletions(-) create mode 100644 .style.yapf create mode 100644 .yapfignore create mode 100755 tools/yapf.sh diff --git a/.style.yapf b/.style.yapf new file mode 100644 index 00000000000..22e59ca28a5 --- /dev/null +++ b/.style.yapf @@ -0,0 +1,421 @@ +####################### +# +# Licensed to the Apache Software Foundation (ASF) under one or more contributor license +# agreements. See the NOTICE file distributed with this work for additional information regarding +# copyright ownership. The ASF licenses this file to you 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 +# +# http://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. +# +####################### + +[style] +based_on_style = yapf + +# Align closing bracket with visual indentation. +align_closing_bracket_with_visual_indent=False + +# Allow dictionary keys to exist on multiple lines. For example: +# +# x = { +# ('this is the first element of a tuple', +# 'this is the second element of a tuple'): +# value, +# } +allow_multiline_dictionary_keys=True + +# Allow lambdas to be formatted on more than one line. +allow_multiline_lambdas=False + +# Allow splitting before a default / named assignment in an argument list. +allow_split_before_default_or_named_assigns=False + +# Allow splits before the dictionary value. +allow_split_before_dict_value=False + +# Let spacing indicate operator precedence. For example: +# +# a = 1 * 2 + 3 / 4 +# b = 1 / 2 - 3 * 4 +# c = (1 + 2) * (3 - 4) +# d = (1 - 2) / (3 + 4) +# e = 1 * 2 - 3 +# f = 1 + 2 + 3 + 4 +# +# will be formatted as follows to indicate precedence: +# +# a = 1*2 + 3/4 +# b = 1/2 - 3*4 +# c = (1+2) * (3-4) +# d = (1-2) / (3+4) +# e = 1*2 - 3 +# f = 1 + 2 + 3 + 4 +# +arithmetic_precedence_indication=False + +# Number of blank lines surrounding top-level function and class +# definitions. +blank_lines_around_top_level_definition=2 + +# Number of blank lines between top-level imports and variable +# definitions. +blank_lines_between_top_level_imports_and_variables=1 + +# Insert a blank line before a class-level docstring. +blank_line_before_class_docstring=False + +# Insert a blank line before a module docstring. +blank_line_before_module_docstring=False + +# Insert a blank line before a 'def' or 'class' immediately nested +# within another 'def' or 'class'. For example: +# +# class Foo: +# # <------ this blank line +# def method(): +# pass +blank_line_before_nested_class_or_def=True + +# Do not split consecutive brackets. Only relevant when +# dedent_closing_brackets is set. For example: +# +# call_func_that_takes_a_dict( +# { +# 'key1': 'value1', +# 'key2': 'value2', +# } +# ) +# +# would reformat to: +# +# call_func_that_takes_a_dict({ +# 'key1': 'value1', +# 'key2': 'value2', +# }) +coalesce_brackets=False + +# The column limit. +column_limit=132 + +# The style for continuation alignment. Possible values are: +# +# - SPACE: Use spaces for continuation alignment. This is default behavior. +# - FIXED: Use fixed number (CONTINUATION_INDENT_WIDTH) of columns +# (ie: CONTINUATION_INDENT_WIDTH/INDENT_WIDTH tabs or +# CONTINUATION_INDENT_WIDTH spaces) for continuation alignment. +# - VALIGN-RIGHT: Vertically align continuation lines to multiple of +# INDENT_WIDTH columns. Slightly right (one tab or a few spaces) if +# cannot vertically align continuation lines with indent characters. +continuation_align_style=SPACE + +# Indent width used for line continuations. +continuation_indent_width=4 + +# Put closing brackets on a separate line, dedented, if the bracketed +# expression can't fit in a single line. Applies to all kinds of brackets, +# including function definitions and calls. For example: +# +# config = { +# 'key1': 'value1', +# 'key2': 'value2', +# } # <--- this bracket is dedented and on a separate line +# +# time_series = self.remote_client.query_entity_counters( +# entity='dev3246.region1', +# key='dns.query_latency_tcp', +# transform=Transformation.AVERAGE(window=timedelta(seconds=60)), +# start_ts=now()-timedelta(days=3), +# end_ts=now(), +# ) # <--- this bracket is dedented and on a separate line +dedent_closing_brackets=False + +# Disable the heuristic which places each list element on a separate line +# if the list is comma-terminated. +disable_ending_comma_heuristic=False + +# Place each dictionary entry onto its own line. +each_dict_entry_on_separate_line=True + +# Require multiline dictionary even if it would normally fit on one line. +# For example: +# +# config = { +# 'key1': 'value1' +# } +force_multiline_dict=False + +# The regex for an i18n comment. The presence of this comment stops +# reformatting of that line, because the comments are required to be +# next to the string they translate. +i18n_comment=#\..* + +# The i18n function call names. The presence of this function stops +# reformattting on that line, because the string it has cannot be moved +# away from the i18n comment. +i18n_function_call=N_, _ + +# Indent blank lines. +indent_blank_lines=False + +# Put closing brackets on a separate line, indented, if the bracketed +# expression can't fit in a single line. Applies to all kinds of brackets, +# including function definitions and calls. For example: +# +# config = { +# 'key1': 'value1', +# 'key2': 'value2', +# } # <--- this bracket is indented and on a separate line +# +# time_series = self.remote_client.query_entity_counters( +# entity='dev3246.region1', +# key='dns.query_latency_tcp', +# transform=Transformation.AVERAGE(window=timedelta(seconds=60)), +# start_ts=now()-timedelta(days=3), +# end_ts=now(), +# ) # <--- this bracket is indented and on a separate line +indent_closing_brackets=False + +# Indent the dictionary value if it cannot fit on the same line as the +# dictionary key. For example: +# +# config = { +# 'key1': +# 'value1', +# 'key2': value1 + +# value2, +# } +indent_dictionary_value=True + +# The number of columns to use for indentation. +indent_width=4 + +# Join short lines into one line. E.g., single line 'if' statements. +join_multiple_lines=False + +# Do not include spaces around selected binary operators. For example: +# +# 1 + 2 * 3 - 4 / 5 +# +# will be formatted as follows when configured with "*,/": +# +# 1 + 2*3 - 4/5 +no_spaces_around_selected_binary_operators= + +# Use spaces around default or named assigns. +spaces_around_default_or_named_assign=False + +# Adds a space after the opening '{' and before the ending '}' dict +# delimiters. +# +# {1: 2} +# +# will be formatted as: +# +# { 1: 2 } +spaces_around_dict_delimiters=False + +# Adds a space after the opening '[' and before the ending ']' list +# delimiters. +# +# [1, 2] +# +# will be formatted as: +# +# [ 1, 2 ] +spaces_around_list_delimiters=False + +# Use spaces around the power operator. +spaces_around_power_operator=False + +# Use spaces around the subscript / slice operator. For example: +# +# my_list[1 : 10 : 2] +spaces_around_subscript_colon=False + +# Adds a space after the opening '(' and before the ending ')' tuple +# delimiters. +# +# (1, 2, 3) +# +# will be formatted as: +# +# ( 1, 2, 3 ) +spaces_around_tuple_delimiters=False + +# The number of spaces required before a trailing comment. +# This can be a single value (representing the number of spaces +# before each trailing comment) or list of values (representing +# alignment column values; trailing comments within a block will +# be aligned to the first column value that is greater than the maximum +# line length within the block). For example: +# +# With spaces_before_comment=5: +# +# 1 + 1 # Adding values +# +# will be formatted as: +# +# 1 + 1 # Adding values <-- 5 spaces between the end of the +# # statement and comment +# +# With spaces_before_comment=15, 20: +# +# 1 + 1 # Adding values +# two + two # More adding +# +# longer_statement # This is a longer statement +# short # This is a shorter statement +# +# a_very_long_statement_that_extends_beyond_the_final_column # Comment +# short # This is a shorter statement +# +# will be formatted as: +# +# 1 + 1 # Adding values <-- end of line comments in block +# # aligned to col 15 +# two + two # More adding +# +# longer_statement # This is a longer statement <-- end of line +# # comments in block aligned to col 20 +# short # This is a shorter statement +# +# a_very_long_statement_that_extends_beyond_the_final_column # Comment <-- the end of line comments are aligned based on the line length +# short # This is a shorter statement +# +spaces_before_comment=2 + +# Insert a space between the ending comma and closing bracket of a list, +# etc. +space_between_ending_comma_and_closing_bracket=False + +# Use spaces inside brackets, braces, and parentheses. For example: +# +# method_call( 1 ) +# my_dict[ 3 ][ 1 ][ get_index( *args, **kwargs ) ] +# my_set = { 1, 2, 3 } +space_inside_brackets=False + +# Split before arguments. +split_all_comma_separated_values=False + +# Split before arguments, but do not split all subexpressions recursively +# (unless needed). +split_all_top_level_comma_separated_values=False + +# Split before arguments if the argument list is terminated by a +# comma. +split_arguments_when_comma_terminated=False + +# Set to True to prefer splitting before '+', '-', '*', '/', '//', or '@' +# rather than after. +split_before_arithmetic_operator=False + +# Set to True to prefer splitting before '&', '|' or '^' rather than +# after. +split_before_bitwise_operator=True + +# Split before the closing bracket if a list or dict literal doesn't fit on +# a single line. +split_before_closing_bracket=True + +# Split before a dictionary or set generator (comp_for). For example, note +# the split before the 'for': +# +# foo = { +# variable: 'Hello world, have a nice day!' +# for variable in bar if variable != 42 +# } +split_before_dict_set_generator=False + +# Split before the '.' if we need to split a longer expression: +# +# foo = ('This is a really long string: {}, {}, {}, {}'.format(a, b, c, d)) +# +# would reformat to something like: +# +# foo = ('This is a really long string: {}, {}, {}, {}' +# .format(a, b, c, d)) +split_before_dot=True + +# Split after the opening paren which surrounds an expression if it doesn't +# fit on a single line. +split_before_expression_after_opening_paren=True + +# If an argument / parameter list is going to be split, then split before +# the first argument. +split_before_first_argument=True + +# Set to True to prefer splitting before 'and' or 'or' rather than +# after. +split_before_logical_operator=False + +# Split named assignments onto individual lines. +split_before_named_assigns=True + +# Set to True to split list comprehensions and generators that have +# non-trivial expressions and multiple clauses before each of these +# clauses. For example: +# +# result = [ +# a_long_var + 100 for a_long_var in xrange(1000) +# if a_long_var % 10] +# +# would reformat to something like: +# +# result = [ +# a_long_var + 100 +# for a_long_var in xrange(1000) +# if a_long_var % 10] +split_complex_comprehension=True + +# The penalty for splitting right after the opening bracket. +split_penalty_after_opening_bracket=300 + +# The penalty for splitting the line after a unary operator. +split_penalty_after_unary_operator=10000 + +# The penalty of splitting the line around the '+', '-', '*', '/', '//', +# `%`, and '@' operators. +split_penalty_arithmetic_operator=300 + +# The penalty for splitting right before an if expression. +split_penalty_before_if_expr=0 + +# The penalty of splitting the line around the '&', '|', and '^' operators. +split_penalty_bitwise_operator=300 + +# The penalty for splitting a list comprehension or generator +# expression. +split_penalty_comprehension=2100 + +# The penalty for characters over the column limit. +split_penalty_excess_character=7000 + +# The penalty incurred by adding a line split to the logical line. The +# more line splits added the higher the penalty. +split_penalty_for_added_line_split=30 + +# The penalty of splitting a list of "import as" names. For example: +# +# from a_very_long_or_indented_module_name_yada_yad import (long_argument_1, +# long_argument_2, +# long_argument_3) +# +# would reformat to something like: +# +# from a_very_long_or_indented_module_name_yada_yad import ( +# long_argument_1, long_argument_2, long_argument_3) +split_penalty_import_names=0 + +# The penalty of splitting the line around the 'and' and 'or' operators. +split_penalty_logical_operator=300 + +# Use the Tab character for indentation. +use_tabs=False + diff --git a/.yapfignore b/.yapfignore new file mode 100644 index 00000000000..591a00b5cdc --- /dev/null +++ b/.yapfignore @@ -0,0 +1,18 @@ +####################### +# +# Licensed to the Apache Software Foundation (ASF) under one or more contributor license +# agreements. See the NOTICE file distributed with this work for additional information regarding +# copyright ownership. The ASF licenses this file to you 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 +# +# http://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. +# +####################### + +lib/* diff --git a/CMakeLists.txt b/CMakeLists.txt index 8329c412cb2..69bfab1de22 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -658,9 +658,20 @@ add_custom_target( VERBATIM ) +# Leave autopep8 for historical reasons, but have it call yapf. It will not do +# to have conflicting Python formatting targets. This can be removed when at +# least CI is updated to use yapf instead of autopep8. add_custom_target( autopep8 - ${CMAKE_SOURCE_DIR}/tools/autopep8.sh ${CMAKE_SOURCE_DIR} + ${CMAKE_SOURCE_DIR}/tools/yapf.sh ${CMAKE_SOURCE_DIR} + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + COMMENT "formatting python files" + VERBATIM +) + +add_custom_target( + yapf + ${CMAKE_SOURCE_DIR}/tools/yapf.sh ${CMAKE_SOURCE_DIR} WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} COMMENT "formatting python files" VERBATIM diff --git a/tools/git/pre-commit b/tools/git/pre-commit index f91cf9e9081..33e9aaa26e5 100755 --- a/tools/git/pre-commit +++ b/tools/git/pre-commit @@ -43,42 +43,41 @@ if [ ! -x "$FORMAT" ]; then exit 1 fi -source "$GIT_TOP/tools/autopep8.sh" -if [ ! -d ${AUTOPEP8_VENV} ]; then - echo "Run \"cmake --build --target autopep8\"" +source "$GIT_TOP/tools/yapf.sh" +if [ ! -d ${YAPF_VENV} ]; then + echo "Run \"cmake --build --target yapf\"" exit 1 fi source "$GIT_TOP/tools/cmake-format.sh" if [ ! -d ${CMAKE_FORMAT_VENV} ]; then - echo "Run \"cmake --build --target autopep8\"" + echo "Run \"cmake --build --target cmake-format\"" exit 1 fi # Where to store the patch clang_patch_file=$(mktemp -t clang-format.XXXXXXXXXX) -autopep8_patch_file=$(mktemp -t autopep8.XXXXXXXXXX) +yapf_patch_file=$(mktemp -t yapf.XXXXXXXXXX) cmake_format_patch_file=$(mktemp -t cmake-format.XXXXXXXXXX) -trap "rm -f $clang_patch_file $autopep8_patch_file $cmake_format_patch_file" 0 1 2 3 5 15 +trap "rm -f $clang_patch_file $yapf_patch_file $cmake_format_patch_file" 0 1 2 3 5 15 # Loop over all files that are changed, and produce a diff file -source ${AUTOPEP8_VENV}/bin/activate +source ${YAPF_VENV}/bin/activate +REPO_ROOT=$(cd $(dirname $0) && git rev-parse --show-toplevel) +YAPF_CONFIG=${REPO_ROOT}/.style.yapf git diff-index --cached --diff-filter=ACMR --name-only HEAD | grep -vE "lib/(catch2|fastlz|swoc|yamlcpp)" | while read file; do case "$file" in *.cc | *.c | *.h | *.h.in) ${FORMAT} "$file" | diff -u "$file" - >>"$clang_patch_file" ;; # Keep this list of Python extensions the same with the list of - # extensions searched for in the toosl/autopep8.sh script. + # extensions searched for in the toosl/yapf.sh script. *.py | *.cli.ext | *.test.ext) - autopep8 \ - --ignore-local-config \ - --exclude ${GIT_TOP}/lib/yamlcpp \ - --max-line-length 132 \ - --aggressive \ - --aggressive \ + yapf \ + --style ${YAPF_CONFIG} \ + --parallel \ --diff \ - "$file" >>"$autopep8_patch_file" + "$file" >>"$yapf_patch_file" ;; esac done @@ -102,14 +101,14 @@ else echo fi -if [ -s "$autopep8_patch_file" ]; then - echo "The commit is not accepted because autopep8 reports issues with it." +if [ -s "$yapf_patch_file" ]; then + echo "The commit is not accepted because yapf reports issues with it." echo "The easiest way to fix this is to run:" echo echo " $ cmake --build --target format" exit 1 else - echo "This commit complies with the current autopep8 formatting rules." + echo "This commit complies with the current yapf formatting rules." echo fi @@ -125,5 +124,5 @@ else fi # Cleanup before exit -rm -f "$clang_patch_file" "$autopep8_patch_file" +rm -f "$clang_patch_file" "$yapf_patch_file" exit 0 diff --git a/tools/yapf.sh b/tools/yapf.sh new file mode 100755 index 00000000000..7a429eba4a2 --- /dev/null +++ b/tools/yapf.sh @@ -0,0 +1,109 @@ +#! /usr/bin/env bash +# +# Simple wrapper to run yapf on a directory. +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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 +# +# http://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. + +# Update these VERSION variables with the new desired yapf tag when a new +# yapf version is desired. +# See: +# https://github.com/google/yapf/tags +YAPF_VERSION="v0.32.0" +VERSION="yapf 0.32.0" + +function main() { + # check for python3 + python3 - << _END_ +import sys + +if sys.version_info.major < 3 or sys.version_info.minor < 8: + exit(1) +_END_ + + if [ $? = 1 ]; then + echo "Python 3.8 or newer is not installed/enabled." + exit 1 + fi + + set -e # exit on error + + if ! type virtualenv >/dev/null 2>/dev/null + then + pip install -q virtualenv + fi + + REPO_ROOT=$(cd $(dirname $0) && git rev-parse --show-toplevel) + YAPF_VENV=${YAPF_VENV:-${REPO_ROOT}/.git/fmt/yapf_${YAPF_VERSION}_venv} + if [ ! -e ${YAPF_VENV} ] + then + python3 -m virtualenv ${YAPF_VENV} + fi + source ${YAPF_VENV}/bin/activate + + pip install -q --upgrade pip + pip install -q "yapf==${YAPF_VERSION}" + + ver=$(yapf --version 2>&1) + if [ "$ver" != "$VERSION" ] + then + echo "Wrong version of yapf!" + echo "Expected: \"${VERSION}\", got: \"${ver}\"" + exit 1 + fi + + DIR=${@:-.} + + # Only run yapf on tracked files. This saves time and possibly avoids + # formatting files the user doesn't want formatted. + tmp_dir=$(mktemp -d -t tracked-git-files.XXXXXXXXXX) + files=${tmp_dir}/git_files.txt + files_filtered=${tmp_dir}/git_files_filtered.txt + git ls-tree -r HEAD --name-only ${DIR} | grep -vE "lib/yamlcpp" > ${files} + # Add to the above any newly added staged files. + git diff --cached --name-only --diff-filter=A >> ${files} + # Keep this list of Python extensions the same with the list of + # extensions searched for in the tools/git/pre-commit hook. + grep -E '\.py$|\.cli.ext$|\.test.ext$' ${files} > ${files_filtered} + # Prepend the filenames with "./" to make the modified file output consistent + # with the clang-format target output. + sed -i'.bak' 's:^:\./:' ${files_filtered} + rm -f ${files_filtered}.bak + + # Efficiently retrieving modification timestamps in a platform + # independent way is challenging. We use find's -newer argument, which + # seems to be broadly supported. The following file is created and has a + # timestamp just before running yapf. Any file with a timestamp + # after this we assume was modified by yapf. + start_time_file=${tmp_dir}/format_start.$$ + touch ${start_time_file} + YAPF_CONFIG=${REPO_ROOT}/.style.yapf + yapf \ + --style ${YAPF_CONFIG} \ + --parallel \ + --in-place \ + $(cat ${files_filtered}) + find $(cat ${files_filtered}) -newer ${start_time_file} + + rm -rf ${tmp_dir} + deactivate +} + +if [[ "$(basename -- "$0")" == 'yapf.sh' ]]; then + main "$@" +else + YAPF_VENV=${YAPF_VENV:-$(git rev-parse --show-toplevel)/.git/fmt/yapf_${YAPF_VERSION}_venv} +fi