Pycodestyle: Enforce 72 character docstring line width

[mblayman]

PEP8 states:

For flowing long blocks of text with fewer structural restrictions (docstrings or comments), the line length should be limited to 72 characters.

The current implementation only enforces the maximum line length (E501). I'd like to suggest the addition of an E503 error to enforce the restriction of these "flowing long blocks of text."

[sigmavirus24]

What does pep257 have to say about this? I would expect that pep would overrule any restrictions here and this change would be automatically turned off by most large projects I know of. In my opinion pep257 is the more appropriate place for this.

[mblayman]

I just read all of PEP257 to be certain and it has nothing to say about docstring line length. Since PEP257 makes no reference to docstring line length and PEP8 does make reference to docstring line length, I must conclude that pep8 would be the correct place to enforce this rule.

[sigmavirus24]

It may be the correct place, but it is far too late to add this as something that's on by default. If it's added, I'm strongly in favor of it being off by default and required to be turned on by developers. Further, I'd be surprised if that line survives much longer once it's pointed out to Guido and the rest of the core developers in charge of maintaining PEP 8, especially given that the maximum line-length is now 99 characters.

[mblayman]

@sigmavirus24 I'm not sure what you mean by "it is far too late to add this as something that's on by default." The PEP states what it expects, and the tool doesn't match that expectation. Since pep8 is a compliance checking tool for PEP8 explicitly, it feels like a bug, regardless of how long the check has been missing from the tool.

As to your other point about the core devs changing their mind, I think more text in the PEP indicates that they considered and rejected the idea that docstrings should be longer.

From PEP8:

For code maintained exclusively or primarily by a team that can reach agreement on this issue, it is okay to increase the nominal line length from 80 to 100 characters (effectively increasing the maximum length to 99 characters), provided that comments and docstrings are still wrapped at 72 characters.

(emphasis added)

I can see how this addition would be inconvenient, but that doesn't mean it's not the right thing to do. If the goal of pep8 is to be 100% compliant with PEP8, then the tool is currently wrong, IMO.

[sigmavirus24]

I can see how this addition would be inconvenient, but that doesn't mean it's not the right thing to do. If the goal of pep8 is to be 100% compliant with PEP8, then the tool is currently wrong, IMO.

It's not inconvenient. It's backwards incompatible. Further there is no where in the README, documentation, or anywhere else that the project states it's the goal to be "100% compliant with PEP8". I'd advise you to stop making false statements along such lines. In fact there are long standing issues about the fact that PEP8 raises warnings and errors about things that are not specified in PEP8. Were its goal to be 100% compliant, it would also enforce the naming conventions put forth in PEP8 and not have these extra checks (as well as enforce other things which it presently doesnt). The goal of this project is not to enforce 100% of PEP8.

[mblayman]

Ah, ok. I did not realize that it was not the goal to be 100% compliant and failed to read "check your Python code against some of the style conventions" in the README, and I made an assumption. That's my mistake. Sorry about that.

Now you've piqued my curiosity: are there PEP8 guidelines that are only optionally activated? The only flag I see in the help that seems related to rule changes is hang-closing. There doesn't seem to be a strict mode or some equivalent.

You make a good point that the suggested change is backwards incompatible (on top of being inconvenient). Is there some policy in place for when changes need to be made that would make the tool more compliant with the spec? How is more comprehensive checking incorporated into the project? Since every new constraint or rule enforcement technically breaks backwards compatibility, I'm unclear on how the tool becomes more accurate over time.

Sorry if my prose was standoffish. I was trying to highlight what I see as an issue and back that with the verbiage from the PEP. Maybe it read as too confrontational. My aim is to help make pep8 as in line with our community standard because I think both are valuable. But I don't want to seem like a jerk in the process.

[sigmavirus24]

Sorry about that

No harm done. It's a common misconception but it's frustrating to disillusion people.

There doesn't seem to be a strict mode or some equivalent.

There isn't. The off-by-default rules currently are controversial rules that some may want/want to enforce.

I'm unclear on how the tool becomes more accurate over time.

It's not easy. As it is I wear many hats in my involvement with the project. One hat is as an OpenStack developer involved in the style enforcing tool for that set of projects. Another is as a user. Another is as the maintainer of Flake8 (with the knowledge that changes in pep8 are reported as bugs there as well). It's hard to juggle all of the interests at once. OpenStack pins the version of pep8 that it uses to avoid the fact that new rules are occasionally added. Most other projects aren't so strict and if they (like OpenStack) integrate flake8 into testing, they may upgrade and their testing/integration systems may start failing unexpectedly.

Sorry if my prose was standoffish. I was trying to highlight what I see as an issue and back that with the verbiage from the PEP. Maybe it read as too confrontational. My aim is to help make pep8 as in line with our community standard because I think both are valuable. But I don't want to seem like a jerk in the process.

It's not my intention to come off as brass either. I work on/contribute to too many projects. I probably need a vacation but there don't seem to be any opportunities ahead for that.

[timj]

I would really welcome this test as we have continuing arguments on our project about docstring lengths (I'd like comment blocks to be restricted to that same length as well) vs other line lengths and distinguishing the two (as is done in PEP8) would be incredibly helpful. I understand backwards compatibility issues but can I suggest that we deal with this by having a new config parameter for max-docstring-length and default it to whatever max-line-length is set to. Then existing users would not even notice a change but people would be able to explicitly set max-docstring-length to 72 or 79 or whatever and see the E503 errors.

[timj]

@sigmavirus24 would you consider accepting a patch as I describe above? This should be a backwards compatible approach.

[sigmavirus24]

@timj would it not be better to add this to pydocstyle?

[timj]

Hmm. This is something covered by PEP8 so it would seem to be relevant to this. Does pydocstyle get called by flake8? @jonathansick do you have an opinion?

[sigmavirus24]

@timj flake8-docstrings is what calls pydocstyle from Flake8. Either way, I'd rather not implement this here if it would be more appropriate in pydocstyle or vice-versa.

[timj]

pycodestyle explicitly states: "pycodestyle (formerly pep8) is a tool to check your Python code against some of the style conventions in PEP 8."

PEP 8 says:

For flowing long blocks of text with fewer structural restrictions (docstrings or comments), the line length should be limited to 72 characters.

so it doesn't seem unreasonable for the program that checks PEP8 compliance to include these line length checks listed in PEP8.

PEP257 (covered by pydocstyle) doesn't mention line lengths and doesn't mention comment strings. This strongly suggests to me that pycodestyle should be dealing with warnings about docstring and comment line length.

[sigmavirus24]

@timj exactly, against some of the style conventions. Just because a convention exists in the document does not mean it needs to be implemented here. For example, the naming conventions live in a Flake8 plugin called pep8-naming and only strives (but barely accomplishes) implementing checks for that. pydocstyle likewise does not only implement PEP 257 checks as it allows for other conventions and is very configurable. It also has the benefit of directly inspecting docstrings via the AST and that should allow it to discern this more readily that pycodestyle would. In other words, to implement it there should be far less work. So while it's not the right place by vague document relations it probably is the best place for accuracy, relevance to users, and ease of implementation.

[timj]

Does pydocstyle recognize comments as well as docstrings?

[brianv0]

I've got a PR in for this if anyone is interested. I hoped to meet everyone's needs in it, but it might require a bit of discussion.

In short, I don't believe this will impact anyone. I've set the default max-doc-length to 79, but that's mostly so that the code in pycodestyle.py and other tests pass without too much rewriting of both pycodestyle and unit tests (much of the doc length in pycodestyle is over 72 characters).

[timj]

Oh, so you are setting the default to 79 because you are disabling the warning by default. The other options is to default to max-line-length and disable W504 only if you are using that default (since you'll get a standard long line warning as you would before).

[brianv0]

The behavior was updated in the PR. The check is not active (and not on the default ignore list). Instead, the check is only activated when a --max-doc-length=n is entered (or corresponding max_doc_length in config file)

[timj]

I like the approach of the check only activating if you specify a doc length.

[timj]

@sigmavirus24 what do you think of #674?

[McSinyx]

Is there a way to only checks for docstring line width (and ignore comments)? I know that's not what PEP 8 advises but I'd love to have it.

[asottile]

@McSinyx seems there isn't currently, both comments and triple-quoted-strings both trigger W505 (if they were separate codes you could ignore one of them)

[asottile]

The original issue as written is solved by #674 though (and therefore this can be closed)

[McSinyx]

Should I file an issue (or even a PR) on that so this issue can be closed in peace then?

My proposal is to have 3 cases handled:

• Multi-line docstring: max_doc_length, default to max_line_length (current behavior)
• Single-line docstring: max_single_doc_length, default to max_doc_length + 3
• Comment block: max_comment_length, default to max_doc_length

[asottile]

I'm not a maintainer here, but I don't think the complexity of a bunch of new settings is a good idea -- also your current proposal allows single line docstrings to extend beyond max_line_length

[timj]

I agree this should be closed and a new ticket used to discuss more fine-grained control. I'm really happy with the current situation and have no problem forcing my comment lines to match the docstring constraint.

[brianv0]

I would say that it's probably not quite trivial because docstrings are block comments/strings in specific positions in the code (right after function/class definition), so there's positional awareness involved. For normal comments, I think you could do that. Regardless - it needs a new issue.