From c3ca25621b07a226d8beed5f999abb276ec52a84 Mon Sep 17 00:00:00 2001 From: moto <855818+mthrok@users.noreply.github.com> Date: Wed, 7 Jun 2023 21:38:53 -0700 Subject: [PATCH] Merge all the lint/style checks to pre-commit hook (#3414) Summary: Pull Request resolved: https://github.com/pytorch/audio/pull/3414 Differential Revision: D46536717 Pulled By: mthrok fbshipit-source-id: 505bdcdd1b59ca9fe5afc2c8516a0a821e2b8d7e --- .../linux/scripts/run_clang_format.py | 310 ------------------ .../linux/scripts/run_style_checks.sh | 49 --- .flake8 | 12 +- .../workflows/docstring_parameters_sync.yml | 37 --- .github/workflows/lint.yml | 26 +- .github/workflows/stylecheck.yml | 29 -- .pre-commit-config.yaml | 26 +- 7 files changed, 48 insertions(+), 441 deletions(-) delete mode 100755 .circleci/unittest/linux/scripts/run_clang_format.py delete mode 100755 .circleci/unittest/linux/scripts/run_style_checks.sh delete mode 100644 .github/workflows/docstring_parameters_sync.yml delete mode 100644 .github/workflows/stylecheck.yml diff --git a/.circleci/unittest/linux/scripts/run_clang_format.py b/.circleci/unittest/linux/scripts/run_clang_format.py deleted file mode 100755 index 250cc6e387..0000000000 --- a/.circleci/unittest/linux/scripts/run_clang_format.py +++ /dev/null @@ -1,310 +0,0 @@ -#!/usr/bin/env python -"""A wrapper script around clang-format, suitable for linting multiple files -and to use for continuous integration. - -This is an alternative API for the clang-format command line. -It runs over multiple files and directories in parallel. -A diff output is produced and a sensible exit code is returned. - -""" - -import argparse -import codecs -import difflib -import fnmatch -import io -import multiprocessing -import os -import signal -import subprocess -import sys -import traceback -from functools import partial - -try: - from subprocess import DEVNULL # py3k -except ImportError: - DEVNULL = open(os.devnull, "wb") - - -DEFAULT_EXTENSIONS = "c,h,C,H,cpp,hpp,cc,hh,c++,h++,cxx,hxx,cu" - - -class ExitStatus: - SUCCESS = 0 - DIFF = 1 - TROUBLE = 2 - - -def list_files(files, recursive=False, extensions=None, exclude=None): - if extensions is None: - extensions = [] - if exclude is None: - exclude = [] - - out = [] - for file in files: - if recursive and os.path.isdir(file): - for dirpath, dnames, fnames in os.walk(file): - fpaths = [os.path.join(dirpath, fname) for fname in fnames] - for pattern in exclude: - # os.walk() supports trimming down the dnames list - # by modifying it in-place, - # to avoid unnecessary directory listings. - dnames[:] = [x for x in dnames if not fnmatch.fnmatch(os.path.join(dirpath, x), pattern)] - fpaths = [x for x in fpaths if not fnmatch.fnmatch(x, pattern)] - for f in fpaths: - ext = os.path.splitext(f)[1][1:] - if ext in extensions: - out.append(f) - else: - out.append(file) - return out - - -def make_diff(file, original, reformatted): - return list( - difflib.unified_diff( - original, reformatted, fromfile="{}\t(original)".format(file), tofile="{}\t(reformatted)".format(file), n=3 - ) - ) - - -class DiffError(Exception): - def __init__(self, message, errs=None): - super(DiffError, self).__init__(message) - self.errs = errs or [] - - -class UnexpectedError(Exception): - def __init__(self, message, exc=None): - super(UnexpectedError, self).__init__(message) - self.formatted_traceback = traceback.format_exc() - self.exc = exc - - -def run_clang_format_diff_wrapper(args, file): - try: - ret = run_clang_format_diff(args, file) - return ret - except DiffError: - raise - except Exception as e: - raise UnexpectedError("{}: {}: {}".format(file, e.__class__.__name__, e), e) - - -def run_clang_format_diff(args, file): - try: - with io.open(file, "r", encoding="utf-8") as f: - original = f.readlines() - except IOError as exc: - raise DiffError(str(exc)) - invocation = [args.clang_format_executable, file] - - # Use of utf-8 to decode the process output. - # - # Hopefully, this is the correct thing to do. - # - # It's done due to the following assumptions (which may be incorrect): - # - clang-format will returns the bytes read from the files as-is, - # without conversion, and it is already assumed that the files use utf-8. - # - if the diagnostics were internationalized, they would use utf-8: - # > Adding Translations to Clang - # > - # > Not possible yet! - # > Diagnostic strings should be written in UTF-8, - # > the client can translate to the relevant code page if needed. - # > Each translation completely replaces the format string - # > for the diagnostic. - # > -- http://clang.llvm.org/docs/InternalsManual.html#internals-diag-translation - - try: - proc = subprocess.Popen( - invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, encoding="utf-8" - ) - except OSError as exc: - raise DiffError("Command '{}' failed to start: {}".format(subprocess.list2cmdline(invocation), exc)) - proc_stdout = proc.stdout - proc_stderr = proc.stderr - - # hopefully the stderr pipe won't get full and block the process - outs = list(proc_stdout.readlines()) - errs = list(proc_stderr.readlines()) - proc.wait() - if proc.returncode: - raise DiffError( - "Command '{}' returned non-zero exit status {}".format( - subprocess.list2cmdline(invocation), proc.returncode - ), - errs, - ) - return make_diff(file, original, outs), errs - - -def bold_red(s): - return "\x1b[1m\x1b[31m" + s + "\x1b[0m" - - -def colorize(diff_lines): - def bold(s): - return "\x1b[1m" + s + "\x1b[0m" - - def cyan(s): - return "\x1b[36m" + s + "\x1b[0m" - - def green(s): - return "\x1b[32m" + s + "\x1b[0m" - - def red(s): - return "\x1b[31m" + s + "\x1b[0m" - - for line in diff_lines: - if line[:4] in ["--- ", "+++ "]: - yield bold(line) - elif line.startswith("@@ "): - yield cyan(line) - elif line.startswith("+"): - yield green(line) - elif line.startswith("-"): - yield red(line) - else: - yield line - - -def print_diff(diff_lines, use_color): - if use_color: - diff_lines = colorize(diff_lines) - sys.stdout.writelines(diff_lines) - - -def print_trouble(prog, message, use_colors): - error_text = "error:" - if use_colors: - error_text = bold_red(error_text) - print("{}: {} {}".format(prog, error_text, message), file=sys.stderr) - - -def main(): - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument( - "--clang-format-executable", - metavar="EXECUTABLE", - help="path to the clang-format executable", - default="clang-format", - ) - parser.add_argument( - "--extensions", - help="comma separated list of file extensions (default: {})".format(DEFAULT_EXTENSIONS), - default=DEFAULT_EXTENSIONS, - ) - parser.add_argument("-r", "--recursive", action="store_true", help="run recursively over directories") - parser.add_argument("files", metavar="file", nargs="+") - parser.add_argument("-q", "--quiet", action="store_true") - parser.add_argument( - "-j", - metavar="N", - type=int, - default=0, - help="run N clang-format jobs in parallel" " (default number of cpus + 1)", - ) - parser.add_argument( - "--color", default="auto", choices=["auto", "always", "never"], help="show colored diff (default: auto)" - ) - parser.add_argument( - "-e", - "--exclude", - metavar="PATTERN", - action="append", - default=[], - help="exclude paths matching the given glob-like pattern(s)" " from recursive search", - ) - - args = parser.parse_args() - - # use default signal handling, like diff return SIGINT value on ^C - # https://bugs.python.org/issue14229#msg156446 - signal.signal(signal.SIGINT, signal.SIG_DFL) - try: - signal.SIGPIPE - except AttributeError: - # compatibility, SIGPIPE does not exist on Windows - pass - else: - signal.signal(signal.SIGPIPE, signal.SIG_DFL) - - colored_stdout = False - colored_stderr = False - if args.color == "always": - colored_stdout = True - colored_stderr = True - elif args.color == "auto": - colored_stdout = sys.stdout.isatty() - colored_stderr = sys.stderr.isatty() - - version_invocation = [args.clang_format_executable, str("--version")] - try: - subprocess.check_call(version_invocation, stdout=DEVNULL) - except subprocess.CalledProcessError as e: - print_trouble(parser.prog, str(e), use_colors=colored_stderr) - return ExitStatus.TROUBLE - except OSError as e: - print_trouble( - parser.prog, - "Command '{}' failed to start: {}".format(subprocess.list2cmdline(version_invocation), e), - use_colors=colored_stderr, - ) - return ExitStatus.TROUBLE - - retcode = ExitStatus.SUCCESS - files = list_files( - args.files, recursive=args.recursive, exclude=args.exclude, extensions=args.extensions.split(",") - ) - - if not files: - return - - njobs = args.j - if njobs == 0: - njobs = multiprocessing.cpu_count() + 1 - njobs = min(len(files), njobs) - - if njobs == 1: - # execute directly instead of in a pool, - # less overhead, simpler stacktraces - it = (run_clang_format_diff_wrapper(args, file) for file in files) - pool = None - else: - pool = multiprocessing.Pool(njobs) - it = pool.imap_unordered(partial(run_clang_format_diff_wrapper, args), files) - while True: - try: - outs, errs = next(it) - except StopIteration: - break - except DiffError as e: - print_trouble(parser.prog, str(e), use_colors=colored_stderr) - retcode = ExitStatus.TROUBLE - sys.stderr.writelines(e.errs) - except UnexpectedError as e: - print_trouble(parser.prog, str(e), use_colors=colored_stderr) - sys.stderr.write(e.formatted_traceback) - retcode = ExitStatus.TROUBLE - # stop at the first unexpected error, - # something could be very wrong, - # don't process all files unnecessarily - if pool: - pool.terminate() - break - else: - sys.stderr.writelines(errs) - if outs == []: - continue - if not args.quiet: - print_diff(outs, use_color=colored_stdout) - if retcode == ExitStatus.SUCCESS: - retcode = ExitStatus.DIFF - return retcode - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/.circleci/unittest/linux/scripts/run_style_checks.sh b/.circleci/unittest/linux/scripts/run_style_checks.sh deleted file mode 100755 index d6e8ef87fa..0000000000 --- a/.circleci/unittest/linux/scripts/run_style_checks.sh +++ /dev/null @@ -1,49 +0,0 @@ -#!/usr/bin/env bash - -set -eux - -root_dir="$(git rev-parse --show-toplevel)" -conda_dir="${root_dir}/conda" -env_dir="${root_dir}/env" -this_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" - -eval "$("${conda_dir}/bin/conda" shell.bash hook)" -conda activate "${env_dir}" - -# 1. Install tools -conda install -y flake8==3.9.2 -printf "Installed flake8: " -flake8 --version - -clangformat_path="${root_dir}/clang-format" -curl https://oss-clang-format.s3.us-east-2.amazonaws.com/linux64/clang-format-linux64 -o "${clangformat_path}" -chmod +x "${clangformat_path}" -printf "Installed clang-fortmat" -"${clangformat_path}" --version - -# 2. Run style checks -# We want to run all the style checks even if one of them fail. - -set +e - -exit_status=0 - -printf "\x1b[34mRunning flake8:\x1b[0m\n" -flake8 torchaudio test tools/setup_helpers docs/source/conf.py examples -status=$? -exit_status="$((exit_status+status))" -if [ "${status}" -ne 0 ]; then - printf "\x1b[31mflake8 failed. Check the format of Python files.\x1b[0m\n" -fi - -printf "\x1b[34mRunning clang-format:\x1b[0m\n" -"${this_dir}"/run_clang_format.py \ - -r torchaudio/csrc \ - --clang-format-executable "${clangformat_path}" \ - && git diff --exit-code -status=$? -exit_status="$((exit_status+status))" -if [ "${status}" -ne 0 ]; then - printf "\x1b[31mC++ files are not formatted. Please use clang-format to format CPP files.\x1b[0m\n" -fi -exit $exit_status diff --git a/.flake8 b/.flake8 index b4e3878df3..dae85fef0d 100644 --- a/.flake8 +++ b/.flake8 @@ -1,4 +1,12 @@ [flake8] +# Note: it's recommended to use `pre-commit run -a flake8` + max-line-length = 120 -ignore = E203,E305,E402,E721,E741,F405,W503,W504,F999 -exclude = build,docs/source,_ext,third_party,examples/tutorials +ignore = E203,E402,E741,W503 + +# Note: exclude is not honnored when flake8 is executed from pre-commit. +# pre-commit has a separate config +exclude = build,docs/src,third_party + +per-file-ignores = + examples/tutorials/*.py: E501 diff --git a/.github/workflows/docstring_parameters_sync.yml b/.github/workflows/docstring_parameters_sync.yml deleted file mode 100644 index 398aff1c8d..0000000000 --- a/.github/workflows/docstring_parameters_sync.yml +++ /dev/null @@ -1,37 +0,0 @@ -name: Docstring Parameters Sync -on: - pull_request: - push: - branches: - - nightly - workflow_dispatch: -jobs: - check-docstring-sync: - name: "Check whether the docstring parameters are in-sync" - runs-on: ubuntu-latest - container: - image: pytorch/conda-builder:cpu - steps: - - name: Checkout repository - uses: actions/checkout@v2 - - name: Setup Minconda - uses: conda-incubator/setup-miniconda@v2 - with: - miniconda-version: "latest" - python-version: 3.8 - - name: Create Conda Env - shell: bash -l {0} - run: | - conda clean --all --quiet --yes - CONDA_ENV="${RUNNER_TEMP}/conda_environment_${GITHUB_RUN_ID}" - conda create \ - --yes \ - --prefix "${CONDA_ENV}" \ - "python=3.8" - echo "CONDA_ENV=${CONDA_ENV}" >> "${GITHUB_ENV}" - echo "CONDA_RUN=conda run -p ${CONDA_ENV}" >> "${GITHUB_ENV}" - - name: Run pydocstyle - shell: bash -l {0} - run: | - ${CONDA_RUN} pip install pydocstyle - ${CONDA_RUN} pydocstyle torchaudio diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 8353957280..69d49dc5c9 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -18,20 +18,20 @@ jobs: set -euo pipefail echo '::group::Setup environment' - CONDA_PATH=$(which conda) - eval "$(${CONDA_PATH} shell.bash hook)" - conda create --name ci --quiet --yes python=3.8 pip - conda activate ci - echo '::endgroup::' - - echo '::group::Install lint tools' - pip install --upgrade pip + eval "$("$(which conda)" shell.bash hook)" pip install --progress-bar=off pre-commit + echo '::endgroup::' + set +e - pre-commit run --all-files - - if [ $? -ne 0 ]; then - git --no-pager diff - exit 1 + pre-commit run --all-files --show-diff-on-failure + status=$? + + echo '::group::Add Summry' + if [ $status -ne 0 ]; then + echo '### Lint failure' >> $GITHUB_STEP_SUMMARY + echo '```diff' >> $GITHUB_STEP_SUMMARY + git --no-pager diff >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY fi echo '::endgroup::' + exit $status diff --git a/.github/workflows/stylecheck.yml b/.github/workflows/stylecheck.yml deleted file mode 100644 index 0f83014652..0000000000 --- a/.github/workflows/stylecheck.yml +++ /dev/null @@ -1,29 +0,0 @@ -name: Stylecheck - -on: - pull_request: - push: - branches: - - nightly - - main - - release/* - workflow_dispatch: - -jobs: - python-source-and-configs: - uses: pytorch/test-infra/.github/workflows/linux_job.yml@main - with: - repository: pytorch/audio - docker-image: "pytorch/torchaudio_unittest_base:manylinux" - script: | - set -euo pipefail - - # Set CHANNEL - if [[(${GITHUB_EVENT_NAME} = 'pull_request' && (${GITHUB_BASE_REF} = 'release'*)) || (${GITHUB_REF} = 'refs/heads/release'*) ]]; then - export UPLOAD_CHANNEL=test - else - export UPLOAD_CHANNEL=nightly - fi - - .circleci/unittest/linux/scripts/setup_env.sh - .circleci/unittest/linux/scripts/run_style_checks.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 32280c8f9c..d8c4096aec 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -19,4 +19,28 @@ repos: - black == 22.3 - usort == 1.0.2 - libcst == 0.4.1 - exclude: examples + + - repo: https://github.com/pre-commit/mirrors-clang-format + rev: v11.0.1 + hooks: + - id: clang-format + + - repo: https://github.com/pycqa/flake8 + rev: 4.0.1 + hooks: + - id: flake8 + args: ['torchaudio', 'test', 'tools', 'docs/source/conf.py', 'examples'] + exclude: 'build|docs/src|third_party' + additional_dependencies: + - flake8-breakpoint == 1.1.0 + - flake8-bugbear == 22.6.22 + - flake8-comprehensions == 3.10.0 + - flake8-pyi == 22.5.1 + - mccabe == 0.6.0 + - pycodestyle == 2.8.0 + + - repo: https://github.com/pycqa/pydocstyle + rev: 6.3.0 + hooks: + - id: pydocstyle + exclude: 'build|test|examples|third_party|docs|tools'