Adopt linrtunner as the linting tool #14306
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Todo: the rest of the lints; ort training import order |
| from opgen.ast import * | ||
| from typing import List, Tuple, Union, Optional | ||
| from opgen.lexer import * |
Check notice
Code scanning / CodeQL
'import *' may pollute namespace
There was a problem hiding this comment.
Existing error I didn't have the bandwidth fixing
orttraining/orttraining/test/python/orttraining_test_orttrainer_bert_toy_onnx.py
Fixed
Show fixed
Hide fixed
...est_data/bert_squad_tensorflow2.1_keras2onnx_opset11/generate_tiny_keras2onnx_bert_models.py
Fixed
Show fixed
Hide fixed
| @@ -600,7 +600,7 @@ | |||
| # Use no past state for these models | |||
| if config.use_cache: | |||
| config.use_cache = False | |||
| except: | |||
| except Exception: | |||
Check notice
Code scanning / CodeQL
Empty except
| @@ -102,15 +102,15 @@ | |||
|
|
|||
| try: | |||
| from .build_and_package_info import cuda_version | |||
| except: # noqa | |||
| except Exception: | |||
Check notice
Code scanning / CodeQL
Empty except
Signed-off-by: Justin Chu <[email protected]>
| import numpy as np | ||
|
|
||
| # Restore dlopen flags. | ||
| import orttraining_external_custom_ops |
Check notice
Code scanning / CodeQL
Unused import
edgchen1
reviewed
Jan 17, 2023
|
I'll be back at this after the release to cause less disruption |
This was referenced Feb 1, 2023
Merged
justinchuby
commented
Feb 16, 2023
|
Replaced by #15085 |
justinchuby
added a commit
that referenced
this pull request
Mar 24, 2023
### Description `lintrunner` is a linter runner successfully used by pytorch, onnx and onnx-script. It provides a uniform experience running linters locally and in CI. It supports all major dev systems: Windows, Linux and MacOs. The checks are enforced by the `Python format` workflow. This PR adopts `lintrunner` to onnxruntime and fixed ~2000 flake8 errors in Python code. `lintrunner` now runs all required python lints including `ruff`(replacing `flake8`), `black` and `isort`. Future lints like `clang-format` can be added. Most errors are auto-fixed by `ruff` and the fixes should be considered robust. Lints that are more complicated to fix are applied `# noqa` for now and should be fixed in follow up PRs. ### Notable changes 1. This PR **removed some suboptimal patterns**: - `not xxx in` -> `xxx not in` membership checks - bare excepts (`except:` -> `except Exception`) - unused imports The follow up PR will remove: - `import *` - mutable values as default in function definitions (`def func(a=[])`) - more unused imports - unused local variables 2. Use `ruff` to replace `flake8`. `ruff` is much (40x) faster than flake8 and is more robust. We are using it successfully in onnx and onnx-script. It also supports auto-fixing many flake8 errors. 3. Removed the legacy flake8 ci flow and updated docs. 4. The added workflow supports SARIF code scanning reports on github, example snapshot:  5. Removed `onnxruntime-python-checks-ci-pipeline` as redundant ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> Unified linting experience in CI and local. Replacing #14306 --------- Signed-off-by: Justin Chu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
lintrunneris a linter runner successfully used by pytorch, onnx and onnx-script. This PR adoptslintrunnerto onnxruntime and hand fixed ~2000 flake8 errors in Python code.lintrunnernow runs all required python lints includingflake8,blackandisort. Future lints likeclang-formatcan be added.In particular, this PR removed some very suboptimal patterns:
import *def func(a=[]))not xxx in->xxx not inmembership checksexcept:->except Exception)The added workflow supports SARIF code scanning reports on github, example snapshot:
Motivation and Context
Unified linting experience in CI and local.