Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt linrtunner as the linting tool #14306

Closed
wants to merge 34 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
46127c1
Add lintrunner config
justinchuby Jan 16, 2023
a01d696
Remove obsolete python checks
justinchuby Jan 16, 2023
621e986
requirements
justinchuby Jan 16, 2023
c228bb8
Exclude
justinchuby Jan 16, 2023
343875a
exclude
justinchuby Jan 16, 2023
e25fbb4
Ruff autofix
justinchuby Jan 16, 2023
a3df22c
Ignore E501
justinchuby Jan 16, 2023
5605cba
Update config
justinchuby Jan 16, 2023
da3ebcf
Remove future
justinchuby Jan 16, 2023
028558d
Black format
justinchuby Jan 16, 2023
22038ab
Fix some
justinchuby Jan 16, 2023
e576bd2
More mutable default args
justinchuby Jan 16, 2023
718b123
Bare except
justinchuby Jan 16, 2023
5458526
Fix more
justinchuby Jan 16, 2023
a3ca255
More fixes
justinchuby Jan 16, 2023
e55b6ff
Fix
justinchuby Jan 16, 2023
635669e
typo
justinchuby Jan 16, 2023
d1e55a4
DUO102
justinchuby Jan 16, 2023
213cfb6
Fix init
justinchuby Jan 16, 2023
4b84d82
Fix the rest; fix training import order
justinchuby Jan 16, 2023
a35bfa2
Remove unused imports
justinchuby Jan 16, 2023
86babed
Restore imports with side effects
justinchuby Jan 16, 2023
cc16673
Fix wrap_exception import
justinchuby Jan 17, 2023
ac38c1d
Missing imports
justinchuby Jan 17, 2023
a33b1f8
Remove import stars (uhh)
justinchuby Jan 17, 2023
049838f
undef
justinchuby Jan 17, 2023
5282648
Fix undefined vars
justinchuby Jan 17, 2023
32041c9
Remove unused
justinchuby Jan 17, 2023
711de92
codeql
justinchuby Jan 17, 2023
baf2588
rename to optional
justinchuby Jan 17, 2023
5d1ef15
Delete ReformatSourcePython.bat
justinchuby Jan 17, 2023
ff2ca87
Merge branch 'main' into justinchu/lintrunner
justinchuby Feb 16, 2023
6ee3cf2
Update .lintrunner.toml
justinchuby Feb 16, 2023
d34e790
Update onnxruntime/python/tools/symbolic_shape_infer.py
justinchuby Feb 16, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 5 additions & 23 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,8 @@
max-line-length = 120
per-file-ignores =
__init__.py:F401
format = [flake8 PEP8 ERROR] %(path)s:%(row)d:%(col)d: %(code)s %(text)s
exclude =
# ignore the .git directory
./.git,
# ignore default build directory
./build,
# ignore external dependency files
./cmake/external,
# TODO enable
./docs/python,
# ignore generated flatbuffers code
./onnxruntime/core/flatbuffers/ort_flatbuffers_py,
# TODO enable
./onnxruntime/python/tools,
# ignore test code for now
./onnxruntime/test,
# TODO enable
./orttraining,
# ignore server code for now
./server,
# ignore issues from different git branches
./.git,
ignore = W503, E203
# NOTE: Edit exclude list in .lintrunner.toml

# Ignored codes:
# DUO102: We use random only for math
ignore = W503, E203, E501, DUO102
65 changes: 33 additions & 32 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,11 @@ on:
pull_request:

jobs:
lint-python:
name: Lint Python
optional-lint:
name: Optional Lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: flake8
uses: reviewdog/action-flake8@v3
with:
github_token: ${{ secrets.github_token }}
# Change reviewdog reporter if you need [github-pr-check, github-check, github-pr-review].
reporter: github-pr-check
# Change reporter level if you need.
# GitHub Status Check won't become failure with a warning.
level: error
filter_mode: file
- name: pyflakes
uses: reviewdog/action-pyflakes@v1
with:
github_token: ${{ secrets.github_token }}
reporter: github-pr-check
level: warning
- uses: actions/checkout@v3
- name: misspell # Check spellings as well
uses: reviewdog/action-misspell@v1
with:
Expand Down Expand Up @@ -63,24 +47,41 @@ jobs:
glob_pattern: "**/*.py"

lint-python-format:
# Separated black/isort from other Python linters because we want this job to
# fail and not affect other linters
# According to https://black.readthedocs.io/en/stable/integrations/github_actions.html:
# We recommend the use of the @stable tag, but per version tags also exist if you prefer that.
# Note that the action’s version you select is independent of the version of Black the action will use.
# The version of Black the action will use can be configured via version.
# Required workflow
name: Python format
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
- uses: actions/checkout@v3
- name: Setup Python
uses: actions/setup-python@v4
with:
# Version range or exact version of Python to use, using SemVer's version range syntax. Reads from .python-version if unset.
python-version: "3.10"
- uses: psf/black@stable
- name: Install dependencies
run: |
python -m pip install -r requirements-dev.txt
lintrunner init
- name: Run lintrunner on all files
run: |
set +e
if ! lintrunner --force-color --all-files --tee-json=lint.json; then
echo ""
echo -e "\e[1m\e[36mYou can reproduce these results locally by using \`lintrunner\`.\e[0m"
exit 1
fi
- name: Produce SARIF
if: always()
run: |
python -m lintrunner_adapters to-sarif lint.json lintrunner.sarif
- name: Upload SARIF file
if: always()
continue-on-error: true
uses: github/codeql-action/upload-sarif@v2
with:
options: "--check --diff --color"
version: "22.12.0"
- uses: isort/isort-action@master
# Path to SARIF file relative to the root of the repository
sarif_file: lintrunner.sarif
category: lintrunner
checkout_path: ${{ github.workspace }}

lint-cpp:
name: Lint C++
Expand Down Expand Up @@ -117,7 +118,7 @@ jobs:
name: Lint JavaScript
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3
- uses: reviewdog/action-eslint@v1
with:
reporter: github-pr-check
Expand Down
139 changes: 139 additions & 0 deletions .lintrunner.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
# Configuration for lintrunner https://github.com/suo/lintrunner
# You can install the dependencies and initialize with
#
# ```sh
# pip install lintrunner lintrunner-adapters
# lintrunner init
# ```
#
# This will install lintrunner on your system and download all the necessary
# dependencies to run linters locally.
# If you want to see what lintrunner init will install, run
# `lintrunner init --dry-run`.
#
# To lint local changes:
#
# ```bash
# lintrunner -m main
# ```
#
# To lint all files:
#
# ```bash
# lintrunner --all-files
# ```
#
# To format files:
#
# ```bash
# lintrunner f --all-files
# ```
#
# To read more about lintrunner, see [wiki](https://github.com/pytorch/pytorch/wiki/lintrunner).
# To update an existing linting rule or create a new one, modify this file or create a
# new adapter following examples in https://github.com/justinchuby/lintrunner-adapters.

[[linter]]
code = 'FLAKE8'
include_patterns = [
'**/*.py',
]
exclude_patterns = [
'build/**',
'cmake/external/**',
# TODO enable
'docs/python/**',
# ignore generated flatbuffers code
'onnxruntime/core/flatbuffers/ort_flatbuffers_py/**',
# TODO enable
'./onnxruntime/python/tools/**',
# FIXME(#7032): ignore test code for now
'onnxruntime/test/**',
# TODO enable
'orttraining/**',
# FIXME: DUO106
'tools/nuget/generate_nuspec_for_native_nuget.py',
# FIXME: DUO116
'js/scripts/build_web.py',
# FIXME: Too many errors
'onnxruntime/python/tools/tensorrt/perf/**',
]
command = [
'python3',
'-m',
'lintrunner_adapters',
'run',
'flake8_linter',
'--',
'@{{PATHSFILE}}'
]
init_command = [
'python3',
'-m',
'lintrunner_adapters',
'run',
'pip_init',
'--dry-run={{DRYRUN}}',
'flake8==5.0.4',
'flake8-bugbear==22.10.27',
'flake8-pyi==22.10.0',
'dlint==0.13.0',
]

[[linter]]
code = 'BLACK-ISORT'
include_patterns = [
'**/*.py',
]
exclude_patterns = [
'cmake/**',
'orttraining/*',
'onnxruntime/core/flatbuffers/**',
]
command = [
'python3',
'-m',
'lintrunner_adapters',
'run',
'black_isort_linter',
'--',
'@{{PATHSFILE}}'
]
init_command = [
'python3',
'-m',
'lintrunner_adapters',
'run',
'pip_init',
'--dry-run={{DRYRUN}}',
'black==22.10.0',
'isort==5.10.1',
]
is_formatter = true

[[linter]]
code = 'PYLINT'
include_patterns = [
# TODO: Opt in to pylint by adding paths here
]
exclude_patterns = [
]
command = [
'python3',
'-m',
'lintrunner_adapters',
'run',
'pylint_linter',
'--rcfile=pyproject.toml',
'--',
'@{{PATHSFILE}}'
]
init_command = [
'python3',
'-m',
'lintrunner_adapters',
'run',
'pip_init',
'--dry-run={{DRYRUN}}',
'pylint==2.15.5',
]
47 changes: 41 additions & 6 deletions docs/Coding_Conventions_and_Standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@ void foo(gsl::span<const std::string> names) {
* The following C++ warnings should never be disabled in onnxruntime VC++ projects(Required by [Binskim](https://github.com/microsoft/binskim/blob/d9afb65c89a621411efded74c27999281d87867e/src/BinSkim.Rules/PERules/BA2007.EnableCriticalCompilerWarnings.cs)).
1. [4018](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4018) 'token' : signed/unsigned mismatch
2. [4146](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4146?view=msvc-160) unary minus operator applied to unsigned type, result still unsigned
3. [4244](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4244?view=msvc-160) 'argument' : conversion from 'type1' to 'type2', possible loss of data. For example, casting a int64_t to size_t.
3. [4244](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4244?view=msvc-160) 'argument' : conversion from 'type1' to 'type2', possible loss of data. For example, casting a int64_t to size_t.
4. [4267](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4267?view=msvc-160) 'var' : conversion from 'size_t' to 'type', possible loss of data.
5. [4302](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4302?view=msvc-160) 'conversion' : truncation from 'type 1' to 'type 2'
6. [4308](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4308?view=msvc-160) negative integral constant converted to unsigned type
6. [4308](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4308?view=msvc-160) negative integral constant converted to unsigned type
7. [4532](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4532?view=msvc-160) 'continue' : jump out of \_\_finally/finally block has undefined behavior during termination handling
8. [4533](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4533?view=msvc-160) initialization of 'variable' is skipped by 'instruction'
9. [4700](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-and-level-4-c4700?view=msvc-160) uninitialized local variable 'name' used
10. [4789](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4789?view=msvc-160) buffer 'identifier' of size N bytes will be overrun; M bytes will be written starting at offset L
11. [4995](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4995?view=msvc-160) 'function': name was marked as #pragma deprecated
11. [4995](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4995?view=msvc-160) 'function': name was marked as #pragma deprecated
12. [4996](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-160) Your code uses a function, class member, variable, or typedef that's marked deprecated

#### Clang-format
Expand Down Expand Up @@ -150,6 +150,42 @@ There is a configuration file in `onnxruntime/VSCodeCoverage.runsettings` that c

Using `Show Code Coverage Coloring` will allow you to visually inspect which lines were hit by the tests. See <https://docs.microsoft.com/en-us/visualstudio/test/using-code-coverage-to-determine-how-much-code-is-being-tested?view=vs-2017>.

## Linting

This project uses [lintrunner](https://github.com/suo/lintrunner) for linting. It provides a consistent linting experience locally and in CI. You can install the dependencies and initialize with

```sh
pip install lintrunner lintrunner-adapters
lintrunner init
```

This will install lintrunner on your system and download all the necessary
dependencies to run linters locally.
If you want to see what lintrunner init will install, run
`lintrunner init --dry-run`.

To lint local changes:

```bash
lintrunner -m main
```

To lint all files:

```bash
lintrunner --all-files
```

To format files:

```bash
lintrunner -a --all-files
```

To read more about lintrunner, see [wiki](https://github.com/pytorch/pytorch/wiki/lintrunner).
To update an existing linting rule or create a new one, modify `.lintrunner.toml` or create a
new adapter following examples in https://github.com/justinchuby/lintrunner-adapters.

## Python Code Style

Follow the [Black formatter](https://black.readthedocs.io)'s coding style when possible. A maximum line length of 120 characters is allowed for consistency with the C++ code.
Expand All @@ -160,11 +196,10 @@ Code can be validated with [flake8](https://pypi.org/project/flake8/) using the

Use `pyright`, which is provided as a component of the `pylance` extension in VS Code for static type checking.

Auto-formatting is done with `black` and `isort`. The tools are configured in `pyproject.toml`. From anywhere in the repository, you can run
Auto-formatting is done with `black` and `isort`. The tools are configured in `pyproject.toml`. From the root of the repository, you can run

```sh
black .
isort .
lintrunner f --all-files
```

to format Python files.
Expand Down
14 changes: 0 additions & 14 deletions onnxruntime/ReformatSourcePython.bat

This file was deleted.

2 changes: 2 additions & 0 deletions onnxruntime/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
OrtValue,
SparseTensor,
)

# FIXME: Remove star imports
from onnxruntime.capi.training import * # noqa: F403

# TODO: thiagofc: Temporary experimental namespace for new PyTorch front-end
Expand Down
6 changes: 3 additions & 3 deletions onnxruntime/python/backend/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import unittest

import packaging.version
from onnx import ModelProto, helper, version
from onnx import helper, version
from onnx.backend.base import Backend
from onnx.checker import check_model

Expand Down Expand Up @@ -59,7 +59,7 @@ def is_opset_supported(cls, model):
domain = opset.domain if opset.domain else "ai.onnx"
try:
key = (domain, opset.version)
if not (key in helper.OP_SET_ID_VERSION_MAP):
if key not in helper.OP_SET_ID_VERSION_MAP:
error_message = (
"Skipping this test as only released onnx opsets are supported."
"To run this test set env variable ALLOW_RELEASED_ONNX_OPSET_ONLY to 0."
Expand Down Expand Up @@ -124,7 +124,7 @@ def prepare(cls, model, device=None, **kwargs):
raise RuntimeError("Incompatible device expected '{0}', got '{1}'".format(device, get_device()))
return cls.prepare(inf, device, **kwargs)
else:
# type: ModelProto
# ModelProto
# check_model serializes the model anyways, so serialize the model once here
# and reuse it below in the cls.prepare call to avoid an additional serialization
# only works with onnx >= 1.10.0 hence the version check
Expand Down
Loading