pycheckers.py overrides values from flake8 config by passing explicit --ignore=

[posita]

$ cat foo.py
#################################################################################
from __future__ import absolute_import, division, print_function
class FooException(Exception): pass
import os
print('{!r}'.format(os.environ.get('PWD')))
$ cat tox.ini  # one of the places flake8 looks for its config
…
[flake8]  # --------------------------------------------------------------
ignore = E302,E305,E402,E501,E701
# E302 - expected 2 blank lines, found ...
# E305 - expected 2 blank lines after end of function or class
# E402 - module level import not at top of file
# E501 - line too long (... > ... characters)
# E701 - multiple statements on one line (colon)
…
$ flake8 foo.py
$ ~/.emacs.d/elpa/flycheck-pycheckers-20171207.1754/bin/pycheckers.py --checkers flake8 foo.py
WARNING E501:flake8: line too long (81 > 80 characters) at foo.py line 1,81.
WARNING E302:flake8: expected 2 blank lines, found 0 at foo.py line 3,1.
WARNING E701:flake8: multiple statements on one line (colon) at foo.py line 3,30.
WARNING E305:flake8: expected 2 blank lines after class or function definition, found 0 at foo.py line 4,1.
WARNING E402:flake8: module level import not at top of file at foo.py line 4,1.

This looks like it's happening because flake8 is given an explicit --ignore= parameter, which overrides config file values:

$ ipdb3 ~/.emacs.d/elpa/flycheck-pycheckers-20171207.1754/bin/pycheckers.py --checkers flake8 --multi-thread=false foo.py
> /…/.emacs.d/elpa/flycheck-pycheckers-20171207.1754/bin/pycheckers.py(12)<module>()
     11 Later improvements by Marc Sherry <msherry@gmail.com>
---> 12 """
     13

ipdb> b 198
Breakpoint 1 at /…/.emacs.d/elpa/flycheck-pycheckers-20171207.1754/bin/pycheckers.py:198
ipdb> c
> /…/.emacs.d/elpa/flycheck-pycheckers-20171207.1754/bin/pycheckers.py(198)run()
    197             args.append(filename)
1-> 198             process = Popen(
    199                 args, stdout=PIPE, stderr=PIPE, universal_newlines=True,
ipdb> print("'{}'".format("' '".join(args)))
'/usr/bin/env' 'flake8' '--ignore=' '--max-line-length' '80' 'foo.py'
ipdb> q
$ '/usr/bin/env' 'flake8' '--ignore=' '--max-line-length' '80' 'foo.py'
foo.py:1:81: E501 line too long (81 > 80 characters)
foo.py:3:1: E302 expected 2 blank lines, found 0
foo.py:3:30: E701 multiple statements on one line (colon)
foo.py:4:1: E305 expected 2 blank lines after class or function definition, found 0
foo.py:4:1: E402 module level import not at top of file

If one wants to be able customize ignored errors from Emacs, one likely needs a way to differentiate between "don't pass any --ignore argument to the underlying linter" and "pass --ignore= to the underlying linter, because I don't want to ignore anything". I toyed around with the idea of trying to omit the --ignore parameter to pycheckers.py where flycheck-pycheckers-ignore-codes was nil (and optionally setting its default to nil), but I'm not sure if this is the right approach. That would likely require changes to both flycheck-pycheckers.el and pycheckers.py.

[msherry]

Thanks for filing the issue! I don't use flake8 much myself anymore, so I'm not sure that I've observed this specific bug, but it definitely sounds like it could apply to other checkers, as well.

I'll definitely take a look at this as soon as I can. Can you confirm that, if you just comment out the

'--ignore=' + ','.join(self.ignore_codes),

line in pycheckers.py, that the checker works as expected? That will give me a good starting point towards tracking down a workable fix.

[posita]

… Can you confirm that, if you just comment out the

'--ignore=' + ','.join(self.ignore_codes),

line in pycheckers.py, that the checker works as expected? …

Yup! When that line is commented out in Flake8Runner, flake8 appears to honor its config files (e.g., tox.ini). I was able to confirm this by adding/removing values (e.g., E501) to the ignore list in the config file and observing the desired effects when (re)running flycheck-buffer in Emacs. Apologies for not including that in the original report.

I don't use flake8 much myself anymore …

Just curious (not related to this issue), but was that because you found something better, that flake8 provided limited value, or perhaps something else?

[msherry]

Sorry for the delay in getting to this -- work (in languages other than Python) has been getting in the way.

I took a quick look at this last night, and I think we can solve this as you suggest, by giving different options to --ignore different meanings. As a user of this package, it sounds like you would find that passing:

• no --ignore= option at all would fall back to using the config file to find which codes to be ignored,
• --ignore= overrides the config file, ignoring no codes, and,
• --ignore=c1,c2,... overrides the config file, ignoring only the codes specified on the command line (or in the flycheck-pycheckers-ignore-codes variable)

Does this match your understanding? If so, I think I can have a fix out pretty quickly.

[posita]

Sorry for the delay in getting to this …

You're not going to get any judgment from me. I could've gotten off my duff and submitted a PR.

… work (in languages other than Python) has been getting in the way.

😏

… Does this match your understanding?

I'm not sure this is responsive, but your description mirrors how I observe flake8 to interpret its --ignore argument:

$ flake8 foo.py  # defers to `ignore` in config
$ flake8 --ignore= foo.py  # includes all errors (i.e., ignores no errors; overrides `ignore` in config)
foo.py:1:80: E501 line too long (81 > 79 characters)
foo.py:3:1: E302 expected 2 blank lines, found 0
foo.py:3:30: E701 multiple statements on one line (colon)
foo.py:4:1: E305 expected 2 blank lines after class or function definition, found 0
foo.py:4:1: E402 module level import not at top of file
$ flake8 --ignore=E302,E501 foo.py  # includes all errors *except* those listed (i.e., ignores only provided errors; overrides `ignore` in config)
foo.py:3:30: E701 multiple statements on one line (colon)
foo.py:4:1: E305 expected 2 blank lines after class or function definition, found 0
foo.py:4:1: E402 module level import not at top of file

Am I answering your question?

[msherry]

Yup, just want to make sure of what the intended behavior should be. Mirroring what existing tools do is a good start. I'll write a fix for this soon-ish.

[msherry]

@posita Want to take a look at this diff and see if it fixes this issue in a way that seems intuitive for you? You'll need to customize-variable flycheck-pycheckers-ignore-codes and choose the "Use options from found config files" option to enable it.

[posita]

Sorry for the delay in my response. This looks like it should do the trick. 👍 (I'm pretty sure I already have flycheck-pycheckers-enable-codes and flycheck-pycheckers-ignore-codes set to nil via custom-set-variables in my .emacs, so I should see the changes post-update.)