New release?

[antoine-gallix]

Is there a planned date for a new release of pycodestyle? This issue for example has been fixed last april, and yet still present on the main release. For direct use of the linter, one can always use the latest master, but it gets complicated to choose pycodestyle version when using it through flake8. It's even worse when using it through even more layers of code, like for example using pre-commit. For example, in our project, we use pre-commit to run black formatter, and then flake8. Black and current flake8-pycodestyle disagree on E252 in function signature with type annotation and we had to silence errors to be able to commit. And as a more general question, what is the release policy of this package?

[asottile]

We can probably coordinate with the planned pyflakes + flake8 release happening in https://github.com/PyCQA/pyflakes/pull/412

At the very least someone would need to put together the changelog for pycodestyle

P.S. with pre-commit you can use a git revision of pycodestyle using additional_dependencies.

For instance:

repos:
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v2.1.0
    hooks:
    -   id: flake8
        additional_dependencies: [
            'git+https://github.com/pycqa/pycodestyle@9f8240e5',
        ]

[antoine-gallix]

Oh I didn't thought that keyword could be used for this purpose. Thanks, that solves our problem until the next flake8/pycodestyle release.

[sigmavirus24]

At the very least someone would need to put together the changelog for pycodestyle

Right. I can cut a release. I wonder if there are any open PRs hanging out, though, that are reasonable blockers

[asottile]

I'm going to check the latest master against a bunch of repos and see if there's anything outstanding / breaking

[asottile]

well right off the bat I'm noticing a trend:

# while checking flake8
(320 similar lines)
+flake8/tests/unit/test_option_manager.py:132: W505 doc line too long (77 > 72 characters)
+flake8/tests/unit/test_option_manager.py:160: W505 doc line too long (77 > 72 characters)
+flake8/tests/unit/test_option_manager.py:46: W505 doc line too long (73 > 72 characters)
+flake8/tests/unit/test_option_manager.py:81: W505 doc line too long (73 > 72 characters)
+flake8/tests/unit/test_option_manager.py:93: W505 doc line too long (76 > 72 characters)
+flake8/tests/unit/test_plugin.py:12: W505 doc line too long (73 > 72 characters)
+flake8/tests/unit/test_plugin.py:21: W505 doc line too long (76 > 72 characters)
+flake8/tests/unit/test_plugin_manager.py:41: W505 doc line too long (76 > 72 characters)
+flake8/tests/unit/test_plugin_type_manager.py:197: W505 doc line too long (73 > 72 characters)
+flake8/tests/unit/test_statistics.py:76: W505 doc line too long (75 > 72 characters)
+flake8/tests/unit/test_style_guide.py:36: W505 doc line too long (73 > 72 characters)
+flake8/tests/unit/test_style_guide.py:70: W505 doc line too long (73 > 72 characters)
+flake8/tests/unit/test_style_guide.py:91: W505 doc line too long (73 > 72 characters)
+flake8/tests/unit/test_utils.py:129: W505 doc line too long (73 > 72 characters)
+flake8/tests/unit/test_utils.py:154: W505 doc line too long (78 > 72 characters)
+flake8/tests/unit/test_utils.py:75: W505 doc line too long (75 > 72 characters)
+flake8/tests/unit/test_violation.py:31: W505 doc line too long (77 > 72 characters)
+flake8/tests/unit/test_violation.py:39: W505 doc line too long (76 > 72 characters)

W505 is going to break the world -- I don't think it belongs in pycodestyle, probably should be in pydocstyle if at all.

If we proceed with it still in pycodestyle I suspect it should be disabled by default, but it should probably be reverted and added instead to pydocstyle: https://github.com/PyCQA/pycodestyle/pull/674

[asottile]

Even in the default ignore it's quite annoying as setting any --ignore will bring that on (vs. flake8's --extend-ignore)

[asottile]

Disabling W505, here's the difference for my repos (which appears to be a bugfix \o/):

-output/asottile/reorder_python_imports/test_data/inputs/no_eol.py:2: W391 blank line at end of file
+output/asottile/reorder_python_imports/test_data/inputs/no_eol.py:2: W292 no newline at end of file

I'm using this script:

import difflib
import os.path
import subprocess
import sys

HERE = os.path.abspath(os.path.dirname(__file__))
OLD = os.path.join(HERE, 'old/bin/flake8')
NEW = os.path.join(HERE, 'new/bin/flake8')

def _output(f):
    res = subprocess.run(
        (f, sys.argv[1], '--format=%(path)s:%(row)d: %(code)s %(text)s'),
        encoding='UTF-8',
        stdout=subprocess.PIPE,
        stderr=subprocess.STDOUT,
    )
    return sorted(res.stdout.splitlines(True))

def main():
    before = _output(OLD)
    after = _output(NEW)
    diff = difflib.unified_diff(before, after, fromfile='old', tofile='new')
    diff = tuple(diff)
    if diff:
        print('*' * 79)
        print(sys.argv[1])
        print('*' * 79)
    for line in diff:
        print(line, end='')

if __name__ == '__main__':
    exit(main())

And all-repos:

all-repos-find-files --repos '\.py$' | xargs -n1 python3 /tmp/t/diff.py

[sigmavirus24]

W505 is going to break the world -- I don't think it belongs in pycodestyle, probably should be in pydocstyle if at all.

I'd want @PyCQA/pydocstyle-dev buy-in on that migration before actually doing it. That said, I'm 90% certain that pydocstyle only works via AST and thus loses line-length information that would be valuable here. That said, I could see this being added to flake8-docstrings as I'm the only person who works on that and while it's entirely a proxy to pydocstyle, it could easily grow extra checks.

I also think that it's totally normal for things like this to happen with pycodestyle. Having it grow the same options as Flake8 in parallel and duplicating the code is a nightmare that I don't want to live in. So if this pushes more people to consuming pycodestyle via Flake8 using --extend-ignore that's fine by me. Worst case scenario, people are not capping their dependencies, get the new one and have to push a 1-line change to their config file to ignore this by default. That's horrible, but it's the reality that people who use this tool must accept and have accepted for years.

[asottile]

right, but if it's annoying out of the box people will drop it like I drop pylint 😆 (I kid I kid, pylint is ok once you write out a config file that disables most of the checks)

[asottile]

Looking of pytest-dev, here's the only interesting one:

*******************************************************************************
output/pytest-dev/pytest-selenium
*******************************************************************************
--- old
+++ new
@@ -0,0 +1 @@
+output/pytest-dev/pytest-selenium/testing/conftest.py:45: W605 invalid escape sequence '\w'

https://github.com/pytest-dev/pytest-selenium/blob/61d32ca896718031115f5d42c4aaeedc04605847/testing/conftest.py#L41-L46

Seems like noqa isn't working there any more?

[asottile]

I checked pytest-dev, tox-dev, lyft, and yelp github orgs and other than some repeats of the above three things it looks good.

• I don't think the W391 -> W292 difference is a big deal
• the noqa-failure on W605 seems kinda bad
• and I think we should disable W505 by default

here's the flake8 patch which will be needed: https://gitlab.com/asottile/flake8/commit/10a703beb27b91f57ed24a5e936e8bb6babc946c

[Nurdok]

W505 is going to break the world -- I don't think it belongs in pycodestyle, probably should be in pydocstyle if at all.

I'd want @PyCQA/pydocstyle-dev buy-in on that migration before actually doing it. That said, I'm 90% certain that pydocstyle only works via AST and thus loses line-length information that would be valuable here.

Pydocstyle doesn't use an AST - we tokenize and manually parse the file to preserve all of this non-AST information (the manual parsing part should be refactored to use a 3rd party parser some day). I think it's perfectly reasonable to include a line-length error in pydocstyle, though I suppose it would be off by default since it's not part of PEP-257.

[sigmavirus24]

I suppose it would be off by default since it's not part of PEP-257.

Right, this is why I was eventually swayed to keep this in pycodestyle

[asottile]

Since I wasn't sure why this limit was there to begin with I opened an issue on peps -- hopefully that will clarify this :) https://github.com/python/peps/issues/884

[asottile]

It didn't actually clarify it beyond "FORTRAN does it" and

GVR: If pycodestyle wants to annoy people, this is a good way to do it.

😕

[sigmavirus24]

Yeah, GVR doesn't like this tool, this is why he made up a trademark legal claim to the PEP8 name to make us change the name. :woman_shrugging:

[asottile]

oh for fork's sake :man_facepalming:

[sigmavirus24]

All this to say, as well, that I agree with the 72 column argument. It's fairly well documented that 72 characters per-line reads well for natural text (a reference: https://baymard.com/blog/line-length-readability ).

[asottile]

I bisected the "noqa stops working" bit -- points at this commit: https://github.com/PyCQA/pycodestyle/commit/25221c95fd180fbc30fb17de2c1930386348ac51

I think what's happening here is the error now points in the middle of a token instead of at the beginning or end so flake8 doesn't know to apply the noqa to it?

[sigmavirus24]

I think our noqa handling could be better in Flake8 as well. I don't think we handle # noqa at the end of a multiline expression well, especially when the violation is reported on a separate line from the one where # noqa is physically present.

pycodestyle used to cheat by checking in the function for noqa so that it could just not run the check. We need a better way of doing it and we presently rely heavily on the physical line that we can get out of the file and don't have a good way of communicating a logical line to the StyleGuide in Flake8.

[asottile]

All this to say, as well, that I agree with the 72 column argument. It's fairly well documented that 72 characters per-line reads well for natural text (a reference: https://baymard.com/blog/line-length-readability ).

even if I agreed, I think we can both agree that the initial indent and """ characters shouldn't count towards that limit

[sigmavirus24]

Yeah, I'm less opinionated on the initial indent and """ than I am the actual line length of the text.

That said, I think that also feeds into max-line-length checking anyway. Let's say you have 2 4-space indents + the """ then you have (79 - (2 * 4) - 3) = 68 characters left. So for a first line, I don't think it even matters realistically speaking (if you're using vanilla pycodestyle with the 79 character line-length limit).

I also don't recall how this check works at this point, honestly. So I'm not trying to argue the point specifically, just pointing out that I think the 72 character limit isn't a bad or unfounded idea.

[asottile]

fair enough, pycodestyle implements it as a column limit not a character limit. So if someone is docstringing a method in a class they've got only 61 (72 - 4 * 2 - 3) usable characters before tripping the linter

EDIT: 58 if it's an inline docstring :S

[asottile]

anyway, back on topic

I think the blockers here are:

• either
  • revert https://github.com/PyCQA/pycodestyle/commit/25221c95fd180fbc30fb17de2c1930386348ac51
  • change how flake8 manages noqa
• disable W505 by default (and/or flake8 can just disable W505 by default -- though ideally I'd like to be consistent here)

[sigmavirus24]

I'd prefer to change how we manage noqa since we have an open issue about it.

disable W505 by default (and/or flake8 can just disable W505 by default -- though ideally I'd like to be consistent here)

I'm 90% certain it's already off-by-default here. It would need to be updated in Flake8, yes, to include it there but that's not a blocker to releasing pycodestyle here.

[asottile]

oh! it is off by default but by a mechanism I didn't notice :D (and confused myself from pycodestyle's local settings file)

pycodestyle defaults max_doc_length to None and skips the check if it is None. I guess I should follow that in flake8?

[sigmavirus24]

I thought it was also in the default ignore list... but I'm wrong about that. I think I'd prefer a PR to set the default to 72 and add W505 here: https://github.com/PyCQA/pycodestyle/blob/db80d0750951e4a9d1ac82338285496534d6c061/pycodestyle.py#L84

[asottile]

the current situation would be less painful at least for my use and those which set ignore= (it's ~roughly similar to the setup for mccabe) -- though I'll defer to you on this

[sigmavirus24]

Leaving it as is, makes sense. I think it makes sense for Flake8 at as well to set it to None.

[asottile]

Given that, I think that there's nothing stopping pycodestyle from a release -- I can follow up and fix flake8's noqa behaviour

[asottile]

Actually, looking at this flake8 knows there's a noqa, but reports the error anyway. Looking at the pycodestyle checks there appears to be a pattern of

def checker(..., noqa):
    if noqa:
        return

(for example here: https://github.com/PyCQA/pycodestyle/blob/2e0de4be72335e3a4ed6bf9fe4a2ac48c1e7d5c4/pycodestyle.py#L1388)

should that be happening for W605 as well?

[asottile]

831 fixes that :tada:

[asottile]

All of my concerns have been resolved and the flake8 PR is ready. I think this is good to go!

[sigmavirus24]

I think we still need release notes, but let me see if I can pull those together.

[sigmavirus24]

Released 2.5.0! :tada:

[antoine-gallix]

Great! Thank you guys, that was fast!