Run style checks in Azure Pipelines #1134
Conversation
Run style checks using flake8 and some of its plugins. Add a new `.flake8` configuration file to tweak the style checks. Add a new `check` target in the `Makefile`. Create a new pipeline in Azure to automatically run style checks on every PullRequest. Add new required packages to `environment_test.yml` and to a new `requirements_style.txt`.
|
I expect the style checks to fail big time now, but sets the basis for solving those issues and also choose which style checks we would like to enforce in SimPEG. One major thing I haven't added is |
This way they both run in Azure regardless of one of them failing.
|
I'm still surprised there's a Makefile left over in this repo, It is barely used for anything, and there's no compiled codes in here anyways that would make it that necessary. |
|
@santisoler looks like there's an error in the azure-pipelines file with a bad name for the style check stage https://dev.azure.com/simpeg/simpeg/_build/results?buildId=2110&view=results. Probably needs to be renamed "Style_Check". You can always explicitly set it's displayname though. |
Using spaces in stage names for Azure pipelines cause errors. Rename it using camelcase instead.
I saw @rowanc1's hand in that Makefile. I imagined it's not being heavily used in SimPEG. I usually love to see a Makefile in the repo (even if the repo doesn't involve compilation steps) because it provides an easy way to standardize processes, document them and make them quickly available. For example, instead of adding instructions in the README or in the Contributors Guide on how to run every style check, one could just say "run |
|
We probably want to (for now) allow these tests to fail and still continue with unit tests. |
environment_test.yml
Outdated
| @@ -36,3 +36,10 @@ dependencies: | |||
| - pyvista | |||
| - pip | |||
| - python-kaleido | |||
| # Linters and code style | |||
| - black | |||
There was a problem hiding this comment.
make sure that this version matches the version in the .pre-commit.yml file. (I think 21.7b0)
Pin the version of Black on environment_test.yml and requirements_dev.txt so it matches the one pinned in the pre-commit configuration file.
| - flake8-bugbear | ||
| - flake8-builtins | ||
| - flake8-mutable | ||
| - flake8-rst-docstrings |
There was a problem hiding this comment.
we might also want to use flake8-docstrings for numpy style documentation checking
https://pypi.org/project/flake8-docstrings/
Sure. Let's just merge this PR and fix all the style issues in separate PRs. |
|
Looks like the |
Codecov Report
@@ Coverage Diff @@
## main #1134 +/- ##
=======================================
Coverage 80.07% 80.07%
=======================================
Files 137 137
Lines 20958 20958
=======================================
Hits 16783 16783
Misses 4175 4175 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Thanks @jcapriot for all the changes. I just grouped the target files into a single variable in the Makefile to avoid repetition and to make it easier to change in the future. Feel free to revert it if you don't like it. From my point this PR is ready, so merge it whenever you feel the same. |
|
Awesome, I think the only thing that looks obvious to turn off is the documentation style checking in tests |
Makes sense. We'll never render docstrings of test functions in Sphinx, so we don't care about their format, they just need to be informative when reading them in plain text. |
The pinned version of Black is not compatible with latest version of click, so we need to pin it as well.
Black handles that in its own way and conflicts with what flake8 complains about.
Run style checks using flake8 and some of its plugins. Add a new
.flake8configuration file to tweak the style checks. Add a newchecktarget in theMakefile. Create a new pipeline in Azure toautomatically run style checks on every PullRequest. Add new required
packages to
environment_test.ymland to a newrequirements_style.txt.