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

[Feature Request] Add flake8-comprehensions linter #32

Closed
1 task done
Skylion007 opened this issue Feb 16, 2023 · 3 comments · Fixed by #33 or #34
Closed
1 task done

[Feature Request] Add flake8-comprehensions linter #32

Skylion007 opened this issue Feb 16, 2023 · 3 comments · Fixed by #33 or #34
Assignees
Labels
enhancement New feature or request py Something related to the Python source code
Milestone

Comments

@Skylion007
Copy link
Contributor

Required prerequisites

  • I have searched the Issue Tracker that this hasn't already been reported. (comment there if it has.)

Motivation

Since this project is speed motivated, it would best best use flake8-comprehensions to ensure that any list/dict/tuple etc comprehensions. Skimming over the list, of checks it looks like most if not all the best practices are already implemented in this repo, best just to add it though to ensure that future sub-optimal codepaths are not accidentally introduced.

We recently enabled almost all the checks in the latest version of PyTorch.

Solution

Add flake8-comprehensions plugin

Alternatives

Do not add it, rely on code review to maintain performance and avoid useless or wasteful comprehensions, builtin calls, etc.

Additional context

We enabled all the checks in PyTorch anyhow.

@Skylion007 Skylion007 added the enhancement New feature or request label Feb 16, 2023
@Skylion007 Skylion007 changed the title [Feature Request] : Add flake8-comprehensions (to ensure best speed practices) [Feature Request] : Add flake8-comprehensions linter Feb 16, 2023
@XuehaiPan
Copy link
Member

Thanks for the advice. I'm going to investigate enabling more flake8 plugins.

@XuehaiPan XuehaiPan added this to the 0.8.0 milestone Feb 17, 2023
@XuehaiPan XuehaiPan self-assigned this Feb 17, 2023
@XuehaiPan XuehaiPan added the py Something related to the Python source code label Feb 17, 2023
@XuehaiPan XuehaiPan changed the title [Feature Request] : Add flake8-comprehensions linter [Feature Request] Add flake8-comprehensions linter Feb 17, 2023
@Skylion007
Copy link
Contributor Author

Skylion007 commented Feb 18, 2023

@XuehaiPan FYI, ruff has a pretty good list of plugins (and automated fixes for some): https://beta.ruff.rs/docs/rules/

@XuehaiPan XuehaiPan mentioned this issue Feb 19, 2023
12 tasks
@XuehaiPan
Copy link
Member

@Skylion007 I opened a new PR #34 to resolve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request py Something related to the Python source code
Projects
None yet
2 participants