Allow ignoring specific errors in files #444

Closed asottile closed 3 years ago

In GitLab by @bittner on Sep 8, 2015, 09:28

At the moment it's only possible to ignore

either a complete file (# flake8: noqa on a single line),
a single line (# noqa at the end of a line),
or specific errors in all files being checked (ignore = F403,E501 in a [flake8] section of setup.cfg or tox.ini)

See also the Configuration chapter in the docs.

Enhancement Proposal

1.) Allow ignoring only specific errors in a single file:

# flake8: ignore=F403,E501

While I think this only would be a valuable, almost necessary addition to flake8 already the very same concept could be taken further in addition.

Apply the Same Idea to Lines

2.) Allow ignoring only specific errors on a single line:

# noqa: ignore=F403,E501

Taking the Proposal Further

3.) Allow configuration within certain scopes of code:

Following this syntax it could be used to apply certain (flake8) rules to a code block (e.g. a function). For example, this would allow complexity 12 within the scope of the function my_func:

def my_func():
    # noqa: max-complexity=12
    for i in range(99):
        for j in range(99):
            for k in range(99):
                for m in range(99):
                    pass

In GitLab by @wbond on Sep 8, 2015, 10:56

I would love to see this also, and would be happy to work on a patch if this is something that others think would fit well with the project.

In GitLab by @bittner on Sep 8, 2015, 13:19

I've just seen Pylint has some similar syntax already. It may be a good idea to harmonize the two worlds, maybe match the syntax, or agree on a common syntax. (They even have symbolic names for errors, cool!)

In GitLab by @sigmavirus24 on Sep 27, 2015, 08:37

As you may discover on this related issue I'm very much not in favor of this. I can empathize with why this seems favorable but there are several reasons why I'm against this in Flake8 (let alone pep8):

Flake8 currently has a lot of magic it does to make things behave better in its interactions with pep8
Flake8 would have to completely redesign how pep8 works from the outside
This is actually, in my opinion, not a feature but a problem that generate more headaches that it solves.

Given that the first two points are easily proven by exploring the two projects, I'll expound on the latter:

First let's consider the popularity of Go's fmt command which takes all of the choice (for style) away from the user. It's:

Predictable
Consistent
Non-controversial
Standard
Well liked
Provides constraints around what users can do/configure

We can easily point out examples in other languages that have many different style enforcers, etc. Those are all failing though. Each person has some set of the rules that they dislike or downright hate. Many people see themselves when they use them (and this isn't exclusive of flake8) as fighting with the tool. When you look at pep8, there are tons of personal styles that fit well within pep8 which the tool and the document don't have specific rules or guidelines about. That means that nitpicks and style collisions still happen. There's already many degrees of freedom. With many knobs and that much freedom, there comes a lot of room for problems. pylint offers this, but if you look at a typical pylintrc, you'll find it to be quite large. Entire swaths of their checks are turned off by some and cargo culted by others because there is so much that can be configured. I consider that to be a problem, not a feature.

Currently, Flake8/pep8 allow users to configure things through the command-line and configuration files. This allows a single, uniform, style guide to be created for application to the entire code base. Style guides are meant to provide consistency. This feature undermines that very consistency. If you work on a large project at even a medium sized company, you know there are submodules that are "owned" by certain people. If they dislike pep8 they can just add to that file ignore directives for all of the rules with which they disagree. They can subvert the company's desired style guide and this works especially well in companies where peer review of checked in code is not commonplace or even occasionally performed.

Adding this per-line will also they cause people to ask for you to have entire sections of code ignored with some delimiter on either end of the block because adding the ignore to a specific line isn't simple enough. I agree that # noqa is a hammer, but I've changed my mind about whether that hammer should exist at all or not over the last 3 or 4 years.

Finally, most tools that Flake8 integrates with do not operate on a per-scope basis. The only two that do are mccabe and pyflakes. PyFlakes almost certainly will not accept such a feature and mccabe most certainly will not either. pep8 works on a per-(physical|logical) line basis. Flake8 just glues all of those together (and anything else someone hopes to add in) and gives you a consistent and semi-standardized report.

If a fork of pep8 came around that was a suitable replacement that offered some or all of this, that tool's success would be the proof necessary to convince me that this isn't a terrible idea wrapped in what would seem to be a great piece of functionality.

(Oh and I dread the inevitable yak shaving that would erupt in issues over the minilanguage for configuration.)

In GitLab by @wbond on Sep 27, 2015, 09:01

The only reason I am interested in this is because flake8 chokes on Python2/3 compatible codebases.

I really have no interest in formatting options. I just don't want to litter the # noqa around my codebase because flake8 running on Python 3 freaks out about unicode and other compatibility code, especially since I only want it to ignore the fact that is thinks unicode does not exist. I want all of the other warnings and checks.

Is there some other way to allow flake8 to run in a mode where you can write compatible Python without it getting in a tizzy?

In GitLab by @sigmavirus24 on Sep 27, 2015, 11:22

@wbond can you provide some examples where you're having problems? I think I can make some suggestions but I'd like them to be more useful than "Try x because it's worked for me in the past" when you might not need X.

In GitLab by @bittner on Sep 27, 2015, 13:44

I understand your points. And I do agree. My concerns with the current ignore/exception rules are just two:

Setting a rule globally (either one rule disabled over all files, or all rules disabled over one file) is dangerous for making you overlook serious smells or issues.
Various tools for Python code analysis now have similar but yet still different syntax; I'd love to see a common approach that supports the readability of Python.

While the latter is difficult to fix just in one place (the way we want it), we can at least discuss the former: In general, there should be little to no reason not to follow a sensible guideline. And guidelines are global by nature, that's how they enforce consistency. So, you're right.

In fact, the problem is a different one: How do we make it easy for developers not to disable warnings and still have the code analysis tool's complaints fixed. One solution, partially, is editor support (reformatting code). What else is possible out there? -- Maybe issue #84 comes in handy?

In GitLab by @sigmavirus24 on Sep 27, 2015, 20:03

84 is something that's sort of available through other tools. I'd like to make it something that could be done with flake8 though. I'm still ruminating on it though. Other feedback, ideas, etc. are very welcome on that issue.

I think your question is a very valid one though:

How do we make it easy for developers not to disable warnings and still have the code analysis tool's complaints fixed

I think there are some things that this might forget which is something pep8 doesn't forbid, and that's a personal sense of style. Gofmt never quite gave that option (except to just not run the tool). As I alluded to earlier, there are a few personal styles which easily fit within pep8/flake8. We can certainly start auto-formatting code and that may win us newer users. I don't think that will endear us with many developers who may want to toy with that but have a different style from what we choose to be the default. That way will lead configuration options and other headaches that I think would be worth it (yes worth it).

Another problem we have to acknowledge (which isn't so much a technical problem) is that PEP-0008 and pep8 will continue to change and add new checks. That will cause headaches as new code churns for users. I think we'll want to make sure that before we start recommending this that pep8 be mostly done (I think it is still missing support for some very fundamental recommendations in the document).

In GitLab by @wbond on Oct 6, 2015, 09:16

@sigmavirus24 Sorry, apparently I was off my rocker that day. It does not seem to be an issue to reference Python 2 types such as unicode when running flake8 in Python 3.

The only place I would love to be able to ignore a warning is when I import an identifier from a "private" file into a "public" file to expose in the public API. Often times I run into this pattern when dealing with OS-specific APIs.

In GitLab by @wbond on Oct 6, 2015, 09:35

I guess I spoke too soon. It does choke on long and xrange. Oh well, for now I'll continue to use # noqa, I would just love to find a way to discourage using the "sledge-hammer" in the project.

In GitLab by @sigmavirus24 on Oct 6, 2015, 09:44

@wbond you can specify --builtins if you think those should be part of the language (which they're not) and flake8 vanilla shouldn't be complaining about this if you have those imported into the global namespace.

The only place I would love to be able to ignore a warning is when I import an identifier from a "private" file into a "public" file to expose in the public API. Often times I run into this pattern when dealing with OS-specific APIs.

As in placing something in __all__, e.g.,

from ._myimpl import apifunc

__all__ = ('apifunc',)

In GitLab by @wbond on Oct 6, 2015, 09:52

Here is an example where flake8 is having trouble with Python 2 code: https://github.com/wbond/asn1crypto/blob/master/asn1crypto/util.py#L95.

(I am in the process of ripping out pylint due to licensing issues I was originally unaware of.)

From reading about __all__, I determined using it for silence a lint was a "hack" since it seems to be intended for allowing packages to define what import * meant.

In GitLab by @sigmavirus24 on Oct 6, 2015, 09:56

Not quite. It's useful as an explicit "I'm exporting this as an API".

Also that example:

$ python3.4
Python 3.4.3 (default, May  1 2015, 19:14:18)
[GCC 4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.49)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> long
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'long' is not defined

So again, flake8 (specifically pyflakes) is not wrong. If you don't want warnings about things undefined on Python 3.4 either use --builtins to trick pyflakes (which may not help with the ast module) or don't run flake8 on Python 3.4. We have always only explicitly running flake8 on versions of Python that the code you're writing is written for.

In GitLab by @wbond on Oct 6, 2015, 09:58

Right, this brings up back around to why people want to silence specific errors…

In GitLab by @wbond on Oct 6, 2015, 09:59

Although I haven't figured out why long is an issue for flake8, but unicode is not.

In GitLab by @wbond on Oct 6, 2015, 10:28

Apparently the issue is being somehow clouded by the Sublime Text plugin I am using that is running flake8 checks. Sorry for that. From the command line it does report an error for unicode.

In GitLab by @sigmavirus24 on Oct 6, 2015, 11:36

Right, this brings up back around to why people want to silence specific errors…

Because they're using the tool improperly? That's not a valid reason for this.

If you're writing code for Python 2, run Flake8 under Python 2. If you're writing code with async/await keywords, run it under Python 3.5. We can not and will not support any other way of doing this because it would be an incredible accomplishment to have code written for one version parsed by a version of Python for which the code is incompatible.

In GitLab by @wbond on Oct 6, 2015, 11:39

I am writing code that runs on both. Is that such a strange thing in the Python world?

Again, this is a perfectly valid reason to allow for specific ignores or even special compat flags.

I'm not going to beat this dead horse anymore. It seems you are more interested in being "right" than trying to discuss possible solutions to the types of engineering issues that affect python programmers day-to-day.

In GitLab by @sigmavirus24 on Oct 6, 2015, 11:59

@wbond how is code with things that cause NameErrors on Python 3 writing code that work on both? That's what's not making any sense to me.

I've also provided you an option with --builtins which I'll demonstrate here by using flake8 in a virtualenv:

$ python -V
Python 3.4.3
$ pip install flake8
Collecting flake8
  Downloading flake8-2.4.1-py2.py3-none-any.whl
Collecting pyflakes<0.9,>=0.8.1 (from flake8)
  Using cached pyflakes-0.8.1-py2.py3-none-any.whl
Collecting pep8!=1.6.0,!=1.6.1,!=1.6.2,>=1.5.7 (from flake8)
  Using cached pep8-1.5.7-py2.py3-none-any.whl
Collecting mccabe<0.4,>=0.2.1 (from flake8)
  Using cached mccabe-0.3.1-py2.py3-none-any.whl
Installing collected packages: pyflakes, pep8, mccabe, flake8
Successfully installed flake8-2.4.1 mccabe-0.3.1 pep8-1.5.7 pyflakes-0.8.1
$ cat ex.py
foo = 'bar'
print(unicode(foo))
$ flake8 ex.py
ex.py:2:7: F821 undefined name 'unicode'
$ flake8 --builtins unicode ex.py && echo $?
0
$

If that isn't sufficient I don't understand how this feature is really going to be that much more helpful. Flake8 is written to work on both Python 2 and 3 (as you likely already know) but isn't meant to work across versions. There are ways to work around that but if you don't want to accept them I don't see the point of adding new features that are only going to serve to make a code base less consistent.

In GitLab by @wbond on Oct 6, 2015, 12:09

The code is inside of sys.version_info guards.

Passing builtins is even more of a sledgehammer than noqa, so I have no interest in that.

What I was looking for was a very specific way to say "No really, I know what I am doing here." I don't need it to detect the code is inside of version guards, or anything magical like that.

Honestly, I still don't see why this is unreasonable idea. By having some way to omit a specific check on a specific line, it allows programmers to very clearly communicate that the line is written that way on purpose and that all other errors should be reported. From a conceptual standpoint, this seems like it would be useful when writing cross-version code.

I understand your sentiment for not wanting to encourage littering code-bases with ignore lines. I tried going down that path with pylint an was generally unhappy with the noise in comments. As a result I decided that walking in-step with pep8 is worth it.

And I am not even saying I want you to add this functionality for me. I am more than happy to contribute towards such a goal. However, it seems you are very dead-set against the concept.

In GitLab by @sigmavirus24 on Oct 6, 2015, 12:13

So I'm less opposed to this being for a single line, that said, this still needs to happen in pep8 first (because pep8, believe it or not, actually does 98% of the # noqa parsing and handling). I'm very much opposed to having a line in a single file though.

In GitLab by @wbond on Oct 6, 2015, 12:22

Since the error is F821 this would mean it is coming from pyflakes, correct?

Either way, I suppose I'll poke around some there. Thanks for taking the time to discuss this some more.

In GitLab by @sigmavirus24 on Oct 6, 2015, 12:38

Yeah but pyflakes implements no such ignore/noqa behaviour. That's all how flake8 registers pyflakes with pep8 as a Checker and pep8 implements the noqa logic because pep8 really is the engine for Flake8.

We should try not to conflate two (or more) topics into one thread in the future. The majority of this thread was for discussing the file-wide ignore rule as opposed to the per-line rule so I was interpreting you wanting "this" as wanting to be able to ignore the error on a file-wide basis.

In GitLab by @wbond on Oct 6, 2015, 12:46

Ok, thanks for the pointers.

The original topic was really about allowing specific ignores at multiple different levels. In my situation, as you can probably see now, I am only specifically interested in part 2 of the original post. I thought my post at https://gitlab.com/pycqa/flake8/issues/89#note_2208707 implied that, but in hindsight I understand how you could have read that differently. Lesson learned.

In GitLab by @Suor on Oct 20, 2015, 08:29

I also think file-wide ignores would be beneficial. I agree that it could encourge some people to subvert general style guide. But this works both ways: if you have legacy codebase you can't subvert style to be better. This is how it works now: legacy files are just added to exclude and so will never become better unless you rewrite file entirely.

Probably external configuration will make silently breaking style hard. E.g. no switches within file itself, but somethings like this in setup.cfg:

[flake8:legacy] ; "legacy" is arbitrary here, just a label for people
match = analysis/*,utils/http.py
ignore = E302,E126,E127,T001

Other legitimate use case -- allow using prints in command line utilities, but restrict everywhere else.

In GitLab by @Suor on Oct 21, 2015, 02:13

Another legitimate use case - some math modules that could have variables with single upper letter names 'cause this is how they are written in corresponding papers. Fixing such code is actually making things worse, you'll just loose the correspondense.

In GitLab by @jayvdb on Nov 28, 2015, 15:57

I've built a flake8 extension that does roughly what @Suor describes : https://github.com/jayvdb/flake8-putty

In GitLab by @Suor on Nov 29, 2015, 01:45

Many thanks, John. I'll definitely take a look.

In GitLab by @sigmavirus24 on Mar 28, 2016, 14:22

mentioned in merge request !57

In GitLab by @akvadrako on Oct 25, 2016, 07:06

So what's the conclusion? If this was implemented, I can't find it documented and !57 is huge.

In GitLab by @QCaudron on Nov 17, 2016, 12:23

This seems like a very legitimate proposal, I'm sad to see it's closed. In particular, as mentioned by @Suor, I'm having to litter # noqa everywhere in some mathematical code of N806 and N803, requiring arguments and variable names to be lowercase. I'd appreciate a comment on concluding this enhancement proposal, specifically on what "Option 2" is, and which part of this was pulled into 3.0 ?

In GitLab by @rodrigc on Mar 5, 2017, 12:42

@wbond did you figure out a way for pyflakes to work with a codebase which needs to work on Python 2/3?

I had some code which pyflakes was complaining about:

if sys.version_info >= (3,):
    a = int(35)
else:
    a = long(35)   # pyflakes complains about this line if pyflakes itself is run under Python 3

I couldn't seem to get # noqa to work, so I had to put some hacks in the code like this:

try:
    # Python 2
    long
except NameError:
    # Python 3
    long = None    # for pyflakes

I would have preferred to put a comment in the code for pyflakes, and not add the NameError check.

In GitLab by @sigmavirus24 on Mar 5, 2017, 13:01

@rodrigc what version of Flake8 are you using? If you're getting an F violation from running Flake8 on that code, then # noqa should work. Also this issue is unrelated.

In GitLab by @macie.korte on Mar 15, 2017, 14:50

+1 vote for this issue. We need the same for our project. There are some cases where we have made decisions to suspend certain flake8 rules for certain files and littering the entire file with #noqa tags everywhere is ugly.

Example: We allow import * in our project's environmental settings files because the setting file for each env (production, qa, dev, etc) are expected to override potentially hundreds of settings from the base settings file. Allowing import * is the only graceful way to accomplish this. As a result, we need to specify # noqa: F405 on every line - we feel we should be able to suspend this error for just these files if we wish to.

In GitLab by @uninen on Mar 19, 2017, 02:27

+1 to implement this (1) from me, too, for the same exact reason mentioned above. It's very ugly to litter special (like settings) files with #noqa comments.

In GitLab by @cybergrind on Jul 3, 2017, 13:09

mentioned in merge request !192

In GitLab by @edsouza on Sep 5, 2017, 11:32

Can someone please clarify what the status of this ticket/feature request is?

Was the feature ever implemented, and if so, what is the syntax for using it?

In GitLab by @edsouza on Sep 5, 2017, 11:37

To answer my own question above, it was implemented, and the syntax is, for example:

example = lambda: 'example' # noqa: E731,E123

as documented at http://flake8.pycqa.org/en/3.0.4/user/ignoring-errors.html#in-line-ignoring-errors

In GitLab by @The-Compiler on Nov 26, 2017, 11:28

FWIW there's a flake8-per-file-ignores plugin by Sebastian Noack. That finally made it possible for me to upgrade from flake8 2 :smile:

In GitLab by @etc0de on Nov 13, 2018, 06:01

flake8 throws lots of useless E999 Invalid syntax errors in .pyx files. It seems there is no way to tell it not to do that, without disabling this for .py as well (where it is useful).

@sigmavirus24 can you give some feedback on whether this issue will be possibly reopened? or was it ever implemented?

In GitLab by @asottile on Nov 13, 2018, 07:26

@TonasJ -- for now there's flake8-per-file-ignores (as listed above) for which you can specify *.pyx:E999. In the next release there will be support for a similar thing in flake8 itself (try out the latest master!). There's a few quirks with the configuration format that are being ironed out before release but it should be ~somewhat usable now.

In GitLab by @etc0de on Nov 13, 2018, 13:18

It is good to hear this is coming! That is really all I wanted to know. I can wait, I just mistakenly assumed the issue may have been forgotten.

(I'd rather not use an addon because then it won't work for people just fetching the repo and running python3 setup.py flake8 without any special configs, will it? I prefer waiting for a proper flake8 release and accepting the bogus errors for now, rather than have it be complicated to run for people)

In GitLab by @asottile on Nov 13, 2018, 13:41

Depending on how your setup.py flake8 target works you could have that plugin as a requirement. Generally python setup.py XXX is clunky -- it goes through distutils -- a much more common workflow would be to use tox or some other environment / test manager

In GitLab by @etc0de on Nov 13, 2018, 13:50

I'm aware, but I want to keep tooling dependencies to a minimum for now. setup.py works fine for now for what I'm doing

In GitLab by @sigmavirus24 on Nov 17, 2018, 05:42

I'm aware, but I want to keep tooling dependencies to a minimum for now. setup.py works fine for now for what I'm doing

That may not get you very far. Flake8 is powerful on its own, but most of it's usefulness comes from plugins people have written for it. You should look at the plugins we use ourselves for this to get an idea of how much you could get out of flake8 if you weren't trying to keep tooling dependencies artificially low.

In GitLab by @etc0de on Nov 17, 2018, 06:14

I'll consider it. But so far I feel fine with what I have, but thanks for the pointers. It's just the somewhat useless .pyx syntax errors of flake8 that grate on me, hence me commenting here

FWIW, I've recently come across this issue and even though I'm aware of (and using) flake8's per-file-ignores setting, I'd still love to have a way to specify that information within the file, for two reasons:

It's closer to the code it refers to. It feels OK to keep in mind that a whole project has a central configuration of the details of coding style it enforces, but it's impossible to keep that in mind for individual files. Also, per-file-ignores for single files are a nuisance when moving, renaming or deleting such a file, and are bound to be left behind.
Each tool that wants to pipe a file through flake8 has to go the extra mile of passing the file name along for the per-file-ignores option to apply.

PyCQA / flake8