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

Conventions for linting the code? #457

Open
rht opened this issue Dec 11, 2018 · 4 comments
Open

Conventions for linting the code? #457

rht opened this issue Dec 11, 2018 · 4 comments
Labels

Comments

@rht
Copy link
Contributor

rht commented Dec 11, 2018

I ran flake8 --ignore=F401,E501,E226,E231,W291,E241 quantecon. Found ~405 lint errors excluding the errors I explicitly ignore. There are unused variables and imports scattered around. Would it be fine if I apply yapf to the codebase?

@mmcky
Copy link
Contributor

mmcky commented Jan 21, 2019

@rht do you know any integrations that could apply lint checking on PRs?

@rht
Copy link
Contributor Author

rht commented Jan 21, 2019

There is https://pep8speaks.com/ used by several projects. This takes several seconds to catch lint errors rather than having to wait a few minutes of Travis booting up and running flake8.

@rht
Copy link
Contributor Author

rht commented Jan 21, 2019

Btw, there are lots of whitespace before/after [/]/,/any operator,

  • W291 trailing whitespace
  • E201 whitespace after '['
  • E202 whitespace before ']'
  • E203 whitespace before ','
  • E241 multiple spaces after ','
  • E226 missing whitespace around arithmetic operator

mostly on literal matrices that are intentionally formatted to be that way.
Should these errors be ignored (at the risk of not catching ones that are not false positive) or do I auto-format it with autopep8? I haven't been able to configure yapf to make sure the function definitions are not formatted this way every time

def func(
    ...
):

and so have to resort to autopep8

@mmcky
Copy link
Contributor

mmcky commented Jan 21, 2019

thanks @rht. I have installed pep8speaks to test it out on the next PR that get's submitted.

There are a few violations that we have intentially ignored and whitespace around matrices has been traditionally one of them. pep8 is not always 100% friendly to scientific computing layouts. I would vote to ignore the ones that add value (such as whitespace used to display matrices more nicely).

@oyamad oyamad added the discuss label Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants