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

Transition away from Flake8 for linting #43

Closed
2 of 5 tasks
jack-fs opened this issue Nov 1, 2022 · 4 comments · Fixed by #44
Closed
2 of 5 tasks

Transition away from Flake8 for linting #43

jack-fs opened this issue Nov 1, 2022 · 4 comments · Fixed by #44

Comments

@jack-fs
Copy link
Contributor

jack-fs commented Nov 1, 2022

Linting in CI has broken one too many times. Maybe we should replace flake8

  • Look into flake8 problem
  • Investigate alternatives
  • Determine configuration
  • Update code if need be (hopefully minor)
  • Update GitHub actions
@jack-fs jack-fs changed the title Transition away from Flake8 for listing Transition away from Flake8 for linting Nov 1, 2022
@jack-fs
Copy link
Contributor Author

jack-fs commented Nov 1, 2022

Useful (though outdated?) overview:
https://pawamoy.github.io/posts/python-static-code-analysis-tools/

Tool Notes Pros Cons
prospector wraps PyFlakes, pycodestyle, McCabe, plus lots of extensions Aggregator: a superset of flake8 in spirit No extension ecosystem (is this a con?)
Similar in idea to flake8: wraps pycodestyle, McCabe, but uses PyLint rather than PyFlakes by default Explicitly aims to be less noisy/confusing
Has additional features, broader scope compared to flake8 but no extension ecosystem
pylama Wraps pylint, mypy and others Extensible Seems strictly less compelling than prospector, based on activity and feature set
Ruff Implemented in rust Extremely fast New
Intended to work alongside autoformatters, so stylistic lint rules are de-emphasised intended as flake8 replacement Under very active development
reimplements flake8 features Only a subset of flake8 lint rules
Editor/LSP integration is WIP
Related tools
PyLint More complex checks/suggestions than flake8 Slower than flake8
pycodestyle pep8 checker
mypypyrightpyre/pytype static type checkers
bandit security-focussed; not particularly applicable for our project

@jack-fs
Copy link
Contributor Author

jack-fs commented Nov 1, 2022

tholo/pytest-flake8#87 and its discussion seems to be relevant, given the errors being thrown in CI. One recommended workflow is to run linting/formatting as a pre-commit hook, rather than in CI. This would require us to coordinate to use pre-commit hooks. I don't think this is necessarily an either/or scenario at our scale; and kind of doesn't answer the central problem: the errors being thrown by pytest

@jack-fs jack-fs mentioned this issue Nov 2, 2022
@dsteinberg dsteinberg reopened this Nov 25, 2022
@dsteinberg
Copy link
Contributor

dsteinberg commented Nov 25, 2022

This is failing again - this time with a different problem.....

https://github.com/gradientinstitute/causal-inspection/actions/runs/3545436523/jobs/5953583059

@jack-fs
Copy link
Contributor Author

jack-fs commented Jan 10, 2023

Seems to be due to flake8-quotes: see zheller/flake8-quotes#115. The new version 3.3.2 seems to fix it: see the current CI run https://github.com/gradientinstitute/causal-inspection/actions/runs/3879072775

@jack-fs jack-fs closed this as completed Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants