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

# noqa ignored? #924

Closed
WarrenWeckesser opened this issue Apr 11, 2020 · 13 comments
Closed

# noqa ignored? #924

WarrenWeckesser opened this issue Apr 11, 2020 · 13 comments

Comments

@WarrenWeckesser
Copy link

Here's an example:

import numpy as np


a = np.array([[ 3.0,  1.0, -4.5],
              [-5.1,  1.5,  2.3],
              [ 0.5, -7.7,  7.1]])

pycodestyle reports problems on lines 4 and 6:

$ pycodestyle pycodestyle_array_noqa.py 
pycodestyle_array_noqa.py:4:16: E201 whitespace after '['
pycodestyle_array_noqa.py:6:16: E201 whitespace after '['

I want to ignore those, so I tried

import numpy as np


a = np.array([[ 3.0,  1.0, -4.5],   # noqa
              [-5.1,  1.5,  2.3],   # noqa
              [ 0.5, -7.7,  7.1]])  # noqa

but pycodestyle 2.5.0 seems to ignore those comments. I get the same report when I run pycodestyle on the edited file. Is it possible to get pycodestyle to ignore these whitespace issues?

@WarrenWeckesser
Copy link
Author

WarrenWeckesser commented Apr 11, 2020

After digging a bit more in the documentation, I see that only certain checks can be disabled with # noqa, and E201 is not one of them.

So I guess this is an enhancement request: allow E201 to be disabled with # noqa. In fact, allow any check to be disabled. Provide sensible defaults, but allow me to override all the things!

@asottile
Copy link
Member

for what it's worth, flake8 works properly on this example:

$ flake8 t.py
$ echo $?
0

@WarrenWeckesser
Copy link
Author

WarrenWeckesser commented Apr 11, 2020

I haven't been following the development of the style checkers. Is flake8 a superset of pycodestyle? Should the projects I work on that do limited continuous testing for style switch from pycodestyle to flake8?

@asottile
Copy link
Member

flake8 out of the box is a wrapper around pyflakes and pycodestyle (and mccabe, but that's off-by-default). flake8 also has a plugin ecosystem that allows other rules/linters to be integrated. So in a way it is a superset of pycodestyle, but also provides a bunch of other stuff

@WarrenWeckesser
Copy link
Author

WarrenWeckesser commented Apr 11, 2020

Thanks for the quick response to the issue. Looking forward to pycodestyle 2.6.0!

(Edited--the first version of this comment was heading off-topic.)

@sigmavirus24
Copy link
Member

pycodestyle is more opinionated in what it provides so it doesn't implement support for # noqa for everything. Instead, if you want to have full control, use flake8 as @asottile has already pointed out.

There's a potential future where some of how Flake8 handles these things gets pulled out into a shared resource for pycodestyle but there hasn't been time or enough interest to do it.

@WarrenWeckesser
Copy link
Author

Being opinionated is probably a good thing, but in this case, I'd like to suggest that not having the option to disable E201 and E241 is not desirable, because it can prevent us from improving the readability of our code.

The primary goal of PEP 8 is "readability":

The guidelines provided here are intended to improve the readability of code and make it consistent across the wide spectrum of Python code. As PEP 20 says, "Readability counts".

About "breaking the rules", PEP 8 says

However, know when to be inconsistent -- sometimes style guide recommendations just aren't applicable.

and

Some other good reasons to ignore a particular guideline:

  1. When applying the guideline would make the code less readable, even for someone who is used to reading code that follows this PEP.

"Readability", in general, cannot be encoded in a formatting algorithm. The checks implemented in pycodestyle (and flake8, etc.) are generally pretty good attempts to do so, and in most cases they work really well, but they can never fully capture "readability". E201 and E241 are appropriate checks in many cases, and are reasonable defaults. But there are cases where having these checks enabled forces us to write less readable code.

In NumPy and SciPy, we often have arrays defined with literal data. This is especially common in our unit tests. An array is a grid of values, and the most natural and readable format for such an array is a grid of aligned columns. I gave one example above. Here's another.

I find this format

a = np.array([[ -1.0, 100.0, -1000.0,  1.0],
              [  0.1,   1.0,   -10.0,  1.0],
              [100.0,   0.0,     1.0, -1.0]])

much more readable than this

a = np.array([[-1.0, 100.0, -1000.0, 1.0],
              [0.1, 1.0, -10.0, 1.0],
              [100.0, 0.0, 1.0, -1.0]])

When I read that code, I want it to be easy to see which values are in any row or column. With the compressed spacing enforced, it is difficult to keep track of which numbers make up a column.

I think any style checker that forces us to use the latter format is making a mistake. Using flake8 might be an option, but I'd like to encourage the pycodestyle devs to consider also allowing # noqa for these checks. Allowing the override of more checks is aligned with the goals of PEP 8, as already explained in PEP 8.

@sigmavirus24
Copy link
Member

Using flake8 is the superior option. # noqa on a line disables any check that accepts # noqa and is not discriminating. With flake8, you can tell it which checks you'd like to ignore.

@WarrenWeckesser
Copy link
Author

For completeness: I just found #289, which was motivated by the same issue that I described above.

@larsoner
Copy link

larsoner commented Jul 9, 2020

Opened an issue with flake8 folks, the plugin is maybe workable with some changes to flake8, we'll see...

https://gitlab.com/pycqa/flake8/-/issues/673

@sigmavirus24
Copy link
Member

Allowing the override of more checks is aligned with the goals of PEP 8, as already explained in PEP 8.

As a follow-up since @larsoner revived this issue... This tool implements the checks suggested by PEP 8 but doesn't implement all of them and doesn't have to align itself with all of the constantly changing opinions there. There's not enough time in the day to keep up with that style guide's every whim and concession. Maybe we need to make it clearer that this tool isn't the thing that checks for what the PEP 8 document says today because no tool could reasonably ever do that.

@WarrenWeckesser
Copy link
Author

I consider the pycodestyle issue closed, and plan to encourage SciPy to switch to flake8 in our CI suites, as suggested above, but since you responded to my comment...

There's not enough time in the day to keep up with that style guide's every whim and concession. Maybe we need to make it clearer that this tool isn't the thing that checks for what the PEP 8 document says today because no tool could reasonably ever do that.

No one is asking for that. To implement a true PEP-8 checker in software, you'd probably need a "Jarvis-level" AI that understood the concept of "readability". All I was asking for was a way to manually override a few of the checks, using markup that is already handled by pycodestyle.

@buhtz

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants