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.

Sign up for GitHub

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

Jump to bottom

Enable pylint checks with Ruff and remove pylint from lintrunner #5589

Merged
merged 7 commits into from
Sep 13, 2023
Merged

Enable pylint checks with Ruff and remove pylint from lintrunner #5589

merged 7 commits into from
Sep 13, 2023

Conversation

justinchuby
Copy link
Contributor

@justinchuby justinchuby commented Sep 13, 2023 •
edited
Loading

Description

Remove the pylint linter in favor of the PL rules provided by ruff. This change removes all of the pylint: disable directives in comments and added noqa directives accordingly for ruff.

  • Remove auto format at VS Code save: Removed because they significantly slow down file save when doing a bulk find and replace. Since we have introduced lintrunner autofix, developers can use a single command for formatting which should be easy enough.

Motivation and Context

Ruff has implemented most of the pylint checks we care and it runs much faster.

@justinchuby justinchuby requested review from a team as code owners September 13, 2023 17:12
@justinchuby justinchuby requested a review from jcwchen September 13, 2023 17:12
@justinchuby justinchuby added the topic: code style Issues related to coding style or the linter label Sep 13, 2023
jcwchen
jcwchen reviewed Sep 13, 2023
View reviewed changes
Copy link
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm, so this PR will use Ruff with new enabled checks to replace existing pylint? Will "PLC", "PLE", "PLW" cover all existing pylint?

.vscode/settings.json Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@justinchuby
Add enable pylint checks with Ruff
c740ea0 
Signed-off-by: Justin Chu <justinchu@microsoft.com>
@justinchuby
Remove pylint from linters
7cd9d58 
Signed-off-by: Justin Chu <justinchu@microsoft.com>
@justinchuby
Remove ignores
ad4f5b1 
Signed-off-by: Justin Chu <justinchu@microsoft.com>
@justinchuby
Add noqa
caa023d 
Signed-off-by: Justin Chu <justinchu@microsoft.com>
@justinchuby
Enable PLR
f0a0c44 
Signed-off-by: Justin Chu <justinchu@microsoft.com>
@justinchuby
Fix PLR
486224a 
Signed-off-by: Justin Chu <justinchu@microsoft.com>
@justinchuby justinchuby added the auto update doc Generate md/proto files automatically using the CI pipeline label Sep 13, 2023
@justinchuby
Copy link
Contributor Author

To confirm, so this PR will use Ruff with new enabled checks to replace existing pylint? Will "PLC", "PLE", "PLW" cover all existing pylint?

That's right. I changed to use PL to enable all pylint rules.

@justinchuby
requirements
d98c56c 
Signed-off-by: Justin Chu <justinchu@microsoft.com>
jcwchen
jcwchen approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@justinchuby justinchuby added the review needed: operators approvers Require reviews from members of operators-approvers label Sep 13, 2023
@justinchuby
Copy link
Contributor Author

cc @gramalingam or @xadupre for approval

@justinchuby justinchuby changed the title Add enable pylint checks with Ruff Enable pylint checks with Ruff and remove pylint from the set of lint tools Sep 13, 2023
@justinchuby justinchuby changed the title Enable pylint checks with Ruff and remove pylint from the set of lint tools Enable pylint checks with Ruff and remove pylint from lintrunner Sep 13, 2023
@justinchuby justinchuby added this pull request to the merge queue Sep 13, 2023
@justinchuby justinchuby removed the review needed: operators approvers Require reviews from members of operators-approvers label Sep 13, 2023
Merged via the queue into onnx:main with commit 55ffe4e Sep 13, 2023
@justinchuby justinchuby deleted the justinchu/pylint-ruff branch September 13, 2023 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcwchen jcwchen jcwchen approved these changes

@gramalingam gramalingam gramalingam approved these changes

Assignees
No one assigned
Labels
auto update doc Generate md/proto files automatically using the CI pipeline topic: code style Issues related to coding style or the linter
Projects
PR Tracker
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@justinchuby @gramalingam @jcwchen