Pylint and autopep8 follow slightly different formatting rules

[mfehr]

The format enforced by autopep8 is not exactly the same as the one expected by pylint. This significantly impairs the usability of the python linter, especially for python projects that go beyond single file tools.

[mfehr]

For now I declared the python support as experimental and we should probably deactivate it by default, until someone needs it and is annoyed enough to fix it.

[igilitschenski]

As discussed with @mfehr, I suspect that the reason for this problem is that autopep8 uses pycodestyle. The issues might potentially be solved by using pycodestyle as linter. Together with @wallarelvo, we will look into that.

[igilitschenski]

At the current stage, pycodestyle seems not to support custom indent sizes (the corresponding PR PyCQA/pycodestyle#524 is not merged yet) which seems to be bad for existing code as this linter used 2 by default. Can you provide me with an example where the current autopep8 / linter combination results in the problem described above, so I can take a look at which linter / formatter combination to use?

[mfehr]

@eggerk do you have a good example? I think you got the most unhappy experience with linter + python. :)

[eggerk]

I think most of the issues I had was when the formatter split long lines into two, which most of the time didn't get accepted by the linter.

E.g.

#!/usr/bin/env python

"""docstring"""

from __future__ import print_function


def some_condition_is_true(number, string, boolean):
  """docstring"""
  return number > 0 and len(string) > 0 and boolean


if __name__ == "__main__":
  some_even_longer_condition_that_is_long_somehow = len(__name__) > 0
  if some_condition_is_true(1, 'a',True) and some_even_longer_condition_that_is_long_somehow:
    pass

will be changed to

#!/usr/bin/env python

"""docstring"""

from __future__ import print_function


def some_condition_is_true(number, string, boolean):
  """docstring"""
  return number > 0 and len(string) > 0 and boolean


if __name__ == "__main__":
  some_even_longer_condition_that_is_long_somehow = len(__name__) > 0
  if some_condition_is_true(
      1,
      'a',
          True) and some_even_longer_condition_that_is_long_somehow:
    pass

about which the linter complains:

test.py: C: 18, 0: Wrong hanging indentation (remove 4 spaces).
          True) and some_even_longer_condition_that_is_long_somehow:
      |   ^ (bad-continuation)

It also complains about the name of a variable, but it doesn't tell you why it's wrong:

test.py: C: 14, 2: Invalid constant name "some_even_longer_condition_that_is_long_somehow" (invalid-name)

---

Btw, @fabianbl recently told me about yapf, Google's python formatter, which works better in my experience than the one that's used in here. yapf changes the file to:

#!/usr/bin/env python
"""docstring"""

from __future__ import print_function


def some_condition_is_true(number, string, boolean):
  """docstring"""
  return number > 0 and len(string) > 0 and boolean


if __name__ == "__main__":
  some_even_longer_condition_that_is_long_somehow = len(__name__) > 0
  if some_condition_is_true(
      1, 'a', True) and some_even_longer_condition_that_is_long_somehow:
    pass

which the linter accepts (apart from the variable name).

The linter in this package would again break the if condition:

if __name__ == "__main__":
  some_even_longer_condition_that_is_long_somehow = len(__name__) > 0
  if some_condition_is_true(
          1, 'a', True) and some_even_longer_condition_that_is_long_somehow:
    pass

which gets an error again:

test.py: C: 15, 0: Wrong hanging indentation (remove 4 spaces).
          1, 'a', True) and some_even_longer_condition_that_is_long_somehow:
      |   ^ (bad-continuation)

[igilitschenski]

Overall, there are two ways to resolve this:

1. Switch from autopep8 to yapf
2. Switch from pylint to pycodestyle (once the latter supports more than 80character lines. This seems to be necessary in some asl packages).

For now, I think 1. is the way to go as yapf also provides a pylintrc file.

@mfehr If you agree, I'd see if we can start implementing it.

[mfehr]

Sounds great 👍

[mfehr]

Ah wait, so but does this mean if we switch we will switch the standard as well? Because then it will just reformat everything... Is there a nice way to get the newer version of pycodestyle?

[igilitschenski]

After doing some more research, I realized that the likely source of the bug is that we are probably using two different standards. The way we invoke autopep8 formats for (a modified variant of) pep8 compliance. The pylint config file, on the other hand, seems to use Chromium Style or another modification of the Google Python Style.

Quick fix would be to unify these to Chromium style, which is directly supported by yapf and seems to be closest to what is currently used (e.g. 2 character indents). I'll look into this closer as I am not sure yet. Long term solution is to have some reasonable defaults and make all of this configurable through the newly introduced config files.

[eggerk]

This should have been fixed with #23.