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

Let flake8 check examples #948

Closed
seisman opened this issue Feb 21, 2021 · 8 comments · Fixed by #1477
Closed

Let flake8 check examples #948

seisman opened this issue Feb 21, 2021 · 8 comments · Fixed by #1477
Assignees
Labels
documentation Improvements or additions to documentation maintenance Boring but important stuff for the core devs
Milestone

Comments

@seisman
Copy link
Member

seisman commented Feb 21, 2021

flake8 would check for whitespace and lines >80 (both not caught by black in examples); currently flake8 is only used for pygmt, setup.py, and doc/conf.py.

Originally posted by @meghanrjones in #935 (comment)

Do we want flake8 to check all examples? Or do we want to limit line length in examples?

@seisman
Copy link
Member Author

seisman commented Feb 21, 2021

I don't like flake8 because it still doesn't support pyproject.toml (https://gitlab.com/pycqa/flake8/-/issues/428), but there are alternatives (e.g., https://flakehell.readthedocs.io/)

@seisman seisman added the question Further information is requested label Feb 21, 2021
@weiji14
Copy link
Member

weiji14 commented Feb 21, 2021

Funny, I was looking at flakehell too last week. Dropping flake8 and switching to flakehell might allow us to get rid of the setup.cfg file I think. See also https://dev.to/bowmanjd/using-flake8-and-pyproject-toml-with-flakehell-1cn1

@seisman
Copy link
Member Author

seisman commented Feb 21, 2021

allow us to get rid of the setup.cfg file I think.

Yes!

@seisman seisman added help wanted Helping hands are appreciated and removed question Further information is requested labels Feb 21, 2021
@weiji14
Copy link
Member

weiji14 commented Feb 21, 2021

allow us to get rid of the setup.cfg file I think.

Yes!

See also https://stackoverflow.com/questions/44878600/is-setup-cfg-deprecated 😄. I think it should be safe to remove setup.cfg once (and if) we switch from flake8 to flakehell, but need to be extra careful in case it breaks some obscure Python packaging component.

@seisman seisman self-assigned this Feb 23, 2021
@seisman seisman added this to the 0.3.1 milestone Feb 23, 2021
@seisman seisman removed their assignment Feb 23, 2021
@seisman seisman removed this from the 0.3.1 milestone Feb 23, 2021
@seisman
Copy link
Member Author

seisman commented Feb 23, 2021

Unfortunately, flakehell is no longer maintained.

@weiji14
Copy link
Member

weiji14 commented Feb 23, 2021

Unfortunately, flakehell is no longer maintained.

Oo, that was sudden. Guess we'll need to find or wait for an alternative.

Edit: Would autopep8 help with the original whitespace issue? It also support pyproject.toml according to https://github.com/hhatto/autopep8#pyprojecttoml

@seisman
Copy link
Member Author

seisman commented Feb 25, 2021

As I understand it, flake8 is still the most popular and widely-used tool for style checking. I think we should still use flake8, and (1) wait for the official pyproject.toml support (https://gitlab.com/pycqa/flake8/-/issues/428); (2) see if someone forks and maintains flakehell; (3) see if there are similar wrappers.

Back to the initial issue, I think we should let flake8 check the examples. It should also fix the trailing whitespace issue in #935.

To let flake8 check all examples, we need to change one single line in Makefile:

pygmt/Makefile

Line 11 in e057927

FLAKE8_FILES=$(PROJECT) setup.py doc/conf.py

and fix all warnings.

Before that, we need to discuss #962 first.

@seisman seisman added this to the 0.3.1 milestone Feb 27, 2021
@seisman seisman added the documentation Improvements or additions to documentation label Feb 27, 2021
@weiji14 weiji14 modified the milestones: 0.3.1, 0.4.0 Mar 9, 2021
@weiji14 weiji14 added the maintenance Boring but important stuff for the core devs label Mar 9, 2021
@seisman
Copy link
Member Author

seisman commented Mar 19, 2021

There is an "official" fork of flakehell at https://github.com/flakehell/flakehell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants