QuantEcon.py icon indicating copy to clipboard operation
QuantEcon.py copied to clipboard

Conventions for linting the code?

Open rht opened this issue 5 years ago • 4 comments

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?

rht avatar Dec 11 '18 02:12 rht

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

mmcky avatar Jan 21 '19 05:01 mmcky

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 avatar Jan 21 '19 14:01 rht

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

rht avatar Jan 21 '19 16:01 rht

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).

mmcky avatar Jan 21 '19 22:01 mmcky