From 1684403d73cc9529eccbbcb932d71fb1d27d39cb Mon Sep 17 00:00:00 2001 From: Diptorup Deb Date: Fri, 30 Apr 2021 17:13:42 -0500 Subject: [PATCH] Various code style automation for dpctl 1. `flake8`, `isort` jobs added to check for Python code style for new PRs against `master`, `gold/2021`, `release` branches. 2. `black` max line width changed to 80 chars. 3. `flake8` and `isort` configs added. 4. `pre-commit` hooks added to make it possible to run all these checks prior to pushing code. 5. `CONTRIBUTION.md` updated. --- .flake8 | 28 ++++++++++ .github/workflows/cpp_style_checks.yml | 22 ++++++++ .github/workflows/python_style_checks.yml | 58 ++++++++++++++++++++ .pre-commit-config.yaml | 29 ++++++++++ CONTRIBUTING.md | 64 +++++++++++++++++------ README.md | 1 + pyproject.toml | 12 ++++- setup.cfg | 5 -- 8 files changed, 196 insertions(+), 23 deletions(-) create mode 100644 .flake8 create mode 100644 .github/workflows/cpp_style_checks.yml create mode 100644 .github/workflows/python_style_checks.yml diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000000..bbe8a35244 --- /dev/null +++ b/.flake8 @@ -0,0 +1,28 @@ +[flake8] +filename = *.py, *.pyx +max_line_length = 80 +max-doc-length = 80 +extend-ignore = E203, W503 +show-source = True + +exclude = + versioneer.py + dpctl/_version.py + build + conda.recipe + .git + +per-file-ignores = + dpctl/_sycl_context.pyx: E999, E225, E227 + dpctl/_sycl_device.pyx: E999, E225 + dpctl/_sycl_device_factory.pyx: E999, E225 + dpctl/_sycl_event.pyx: E999, E225 + dpctl/_sycl_platform.pyx: E999, E225 + dpctl/_sycl_queue.pyx: E999, E225, E226, E227 + dpctl/_sycl_queue_manager.pyx: E999, E225 + dpctl/memory/_memory.pyx: E999, E225, E226, E227 + dpctl/program/_program.pyx: E999, E225, E226, E227 + dpctl/tensor/numpy_usm_shared.py: F821 + examples/cython/sycl_buffer/_buffer_example.pyx: E999, E225, E402 + examples/cython/sycl_direct_linkage/_buffer_example.pyx: E999, E225, E402 + examples/cython/usm_memory/blackscholes.pyx: E999, E225, E226, E402 diff --git a/.github/workflows/cpp_style_checks.yml b/.github/workflows/cpp_style_checks.yml new file mode 100644 index 0000000000..b5a7def26f --- /dev/null +++ b/.github/workflows/cpp_style_checks.yml @@ -0,0 +1,22 @@ +# This is a workflow to format C/C++ sources with clang-format + +name: C++ Code Style + +# Controls when the action will run. Triggers the workflow on push or pull request +# events but only for the master branch +on: + pull_request: + push: + branches: [master] + +jobs: + formatting-check: + name: clang-format + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Run clang-format style check for C/C++ programs. + uses: jidicula/clang-format-action@v3.1.0 + with: + clang-format-version: '11' + check-path: 'dpctl-capi' diff --git a/.github/workflows/python_style_checks.yml b/.github/workflows/python_style_checks.yml new file mode 100644 index 0000000000..f48e567625 --- /dev/null +++ b/.github/workflows/python_style_checks.yml @@ -0,0 +1,58 @@ +# This is a workflow to format Python code with black formatter + +name: Python Code Style + +# Controls when the action will run. Triggers the workflow on push or pull request +# events but only for the master branch +on: + pull_request: + push: + branches: [master] + +# A workflow run is made up of one or more jobs that can run sequentially or in parallel +jobs: + # The isort job sorts all imports in .py, .pyx, .pxd files + isort: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-python@v2 + - uses: jamescurtin/isort-action@master + with: + args: ". --check-only" + + black: + # The type of runner that the job will run on + runs-on: ubuntu-latest + + # Steps represent a sequence of tasks that will be executed as part of the job + steps: + # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it + - uses: actions/checkout@v2 + # Set up a Python environment for use in actions + - uses: actions/setup-python@v2 + + # Run black code formatter + - uses: psf/black@21.4b2 + with: + args: ". --check" + + flake8: + runs-on: ubuntu-latest + + strategy: + matrix: + python-version: [3.7] + + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install flake8 + - name: Lint with flake8 + uses: py-actions/flake8@v1 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 51a095222b..0554e90007 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,3 +7,32 @@ repos: - id: bandit pass_filenames: false args: ["-r", "dpctl", "-lll"] +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v2.3.0 + hooks: + - id: end-of-file-fixer + - id: trailing-whitespace +- repo: https://github.com/psf/black + rev: 21.4b2 + hooks: + - id: black + exclude: "versioneer.py|dpctl/_version.py" +- repo: https://github.com/pycqa/isort + rev: 5.8.0 + hooks: + - id: isort + name: isort (python) + - id: isort + name: isort (cython) + types: [cython] + - id: isort + name: isort (pyi) + types: [pyi] +- repo: https://gitlab.com/pycqa/flake8 + rev: 3.9.1 + hooks: + - id: flake8 +- repo: https://github.com/pocc/pre-commit-hooks + rev: v1.1.1 + hooks: + - id: clang-format diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index efa64123c5..c43830c903 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,12 +15,30 @@ Run before each commit: `clang-format -style=file -i dpctl-capi/include/*.h dpct ### Python code style -We use [black](https://black.readthedocs.io/en/stable/) code formatter. +We use the following Python code style tools: +- [black](https://black.readthedocs.io/en/stable/) code formatter. + - Revision: `20.8b1`. +- [flake8](https://flake8.pycqa.org/en/latest/) linter. + - Revision `3.9.1`. +- [isort](https://pycqa.github.io/isort/) import sorter. + - Revision `5.8.0`. -- Revision: `20.8b1`. -- See configuration in `pyproject.toml`. +- Refer `pyproject.toml` and `.flake8` config files for current configurations. -Run before each commit: `black .` +Please run these three tools before each commit. Although, you may choose to +do so manually, but it is much easier and preferable to automate these checks. +Refer your IDE docs to set them up in your IDE, or you can set up `pre-commit` +to add git hooks. + +### Setting up pre-commit + +A `.pre-commit-config.yaml` is included to run various check before you +commit your code. Here are the steps to setup `pre-commit` in your workflow: + +- Install `pre-commit`: `pip install pre-commit` +- Install the git hook scripts: `pre-commit install` + +That should be it! ### C/C++ File Headers @@ -61,8 +79,9 @@ Few things to note about this format: - The copyright year should be updated every calendar year. - Each comment line should be a max of 80 chars. - A Doxygen `\file` tag describing the contents of the file must be provided. - Also note that the `\file` tag is inside a Doxygen comment block (defined by `///` - comment marker instead of the `//` comment marker used in the rest of the header. + Also note that the `\file` tag is inside a Doxygen comment block ( + defined by `///` comment marker instead of the `//` comment marker used in the + rest of the header. ### Python File Headers @@ -91,7 +110,8 @@ The copyright year should be updated every calendar year. ### Bandit -We use [Bandit](https://github.com/PyCQA/bandit) to find common security issues in Python code. +We use [Bandit](https://github.com/PyCQA/bandit) to find common security issues +in Python code. Install: `pip install bandit` @@ -101,18 +121,21 @@ Run before each commit: `bandit -r dpctl -lll` ## Code Coverage -Implement python, cython and c++ file coverage using `coverage` and `llvm-cov` packages on Linux. +Implement python, cython and c++ file coverage using `coverage` and `llvm-cov` +packages on Linux. ### Using Code Coverage -You need to install additional packages and add an environment variable to cover: +You need to install additional packages and add an environment variable to +cover: - conda install cmake - conda install coverage - conda install conda-forge::lcov - conda install conda-forge::gtest - export CODE_COVERAGE=ON -CODE_COVERAGE allows you to build a debug version of dpctl and enable string tracing, which allows you to analyze strings to create a coverage report. +CODE_COVERAGE allows you to build a debug version of dpctl and enable string +tracing, which allows you to analyze strings to create a coverage report. It was added for the convenience of configuring the CI in the future. Installing the dpctl package: @@ -121,19 +144,24 @@ Installing the dpctl package: It is important that there are no files of the old build in the folder. Use `git clean -xdf` to clean up the working tree. -The coverage report will appear during the build in the console. This report contains information about c++ file coverage. -The `dpctl-c-api-coverage` folder will appear in the root folder after installation. -The folder contains a report on the coverage of c++ files in html format. +The coverage report will appear during the build in the console. This report +contains information about c++ file coverage. +The `dpctl-c-api-coverage` folder will appear in the root folder after +installation. The folder contains a report on the coverage of c++ files in html +format. You need to run tests to cover the cython and python files: - coverage run -m unittest dpctl.tests -The required flags for the command coverage run are contained in the file `.coveragerc`. +The required flags for the command coverage run are contained in the file +`.coveragerc`. The simplest reporting is a textual summary produced with report: - coverage report -For each module executed, the report shows the count of executable statements, the number of those statements missed, and the resulting coverage, expressed as a percentage. +For each module executed, the report shows the count of executable statements, +the number of those statements missed, and the resulting coverage, expressed as +a percentage. The `-m` flag also shows the line numbers of missing statements: - coverage report -m @@ -141,14 +169,16 @@ The `-m` flag also shows the line numbers of missing statements: To create an annotated HTML listings with coverage results: - coverage html -The `htmlcov` folder will appear in the root folder of the project. It contains reports on the coverage of python and cython files in html format. +The `htmlcov` folder will appear in the root folder of the project. It contains +reports on the coverage of python and cython files in html format. Erase previously collected coverage data: - coverage erase ### Error in the build process -An error occurs during the dcptl build with the CODE_COVERAGE environment variable: +An error occurs during the dcptl build with the CODE_COVERAGE environment +variable: ``` error: '../compat/unistd.h' file not found, did you mean 'compat/unistd.h'? # include "../compat/unistd.h" diff --git a/README.md b/README.md index bcdd7fb9c5..224176e268 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ [![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black) +[![Imports: isort](https://img.shields.io/badge/%20imports-isort-%231674b1?style=flat&labelColor=ef8336)](https://pycqa.github.io/isort/) What? ==== diff --git a/pyproject.toml b/pyproject.toml index 2b233b61c9..7598e26713 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,2 +1,12 @@ [tool.black] -exclude = 'versioneer.py' +exclude = "versioneer.py|dpctl/_version.py" +line-length = 80 + +[tool.isort] +multi_line_output = 3 +include_trailing_comma = true +force_grid_wrap = 0 +use_parentheses = true +ensure_newline_before_comments = true +line_length = 80 +skip = ["versioneer.py", "dpctl/_version.py"] diff --git a/setup.cfg b/setup.cfg index f360aaf2d3..e754d4e620 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,8 +1,3 @@ -[flake8] -max-line-length = 100 -ignore = E122,E123,E126,E127,E128,E731,E722 -exclude = build,dpctl/_version.py,tests,conda.recipe,.git,versioneer.py,benchmarks,.asv - [tool:pytest] norecursedirs= .* *.egg* build dist conda.recipe addopts =