fix E203 false positive in list slices

[legau]

Resolves #373

[asottile]

trying this out, it doesn't appear to work properly -- consider black's codebase:

$ time python3 -m pycodestyle black/ --exclude black/profiling/,black/tests --ignore=E501,W503,E262,E721,E741 | grep E203
black/src/black/trans.py:653:70: E203 whitespace before ':'
black/src/black/trans.py:727:76: E203 whitespace before ':'

real	0m2.293s
user	0m2.278s
sys	0m0.016s

[legau]

Indeed it didn't work with multi line statements

[asottile]

welcome to open source where you get what you pay for! there are no SLAs and no ETAs

I'm not even sure this should be merged -- the rule is implemented as intended it's mostly that pep8 changed to stop recommending it -- the original rule should be preserved in some way. though it's not urgent to merge this because there's already a working solution for black (as documented in black's documentation)

[lucasmaheo]

I never saw in pep8's documentation that they recommend not using spaces inside slices. However, they did have vague recommendations to not add spaces before colons, with only examples along the lines of if x == 4 :. I believe it dates back to when slicing was not as omnipresent as it is now.

They now (since 2015) have Yes examples with ham[lower+offset : upper+offset] or ham[lower + offset : upper + offset]. Black reflects that in their doc.

I don't think we should just freeze for the sake of history. If it takes complex checks to ensure we are within the current pep8, so be it.

[sigmavirus24]

I never saw in pep8's documentation that they recommend not using spaces inside slices. However, they did have vague recommendations to not add spaces before colons, with only examples along the lines of if x == 4 :. I believe it dates back to when slicing was not as omnipresent as it is now.

Slicing had been widely used since before 2000. maybe you don't remember then but others do. Code exists from then. Styles were adopted and followed since then and enforced with this tool.

They now (since 2015) have Yes examples with ham[lower+offset : upper+offset] or ham[lower + offset : upper + offset]. Black reflects that in their doc.

Yes, you are in agreement with Anthony that this changed. Congratulations

I don't think we should just freeze for the sake of history. If it takes complex checks to ensure we are within the current pep8, so be it.

You're misattributing why we won't change this in a way that seems trollish (deliberately wrong for the purpose of eliciting a response that does what you want).

The reality is that many companies and open source projects have code bases not using black relying on this time e for consistency and compliance. This is why it was suggested to create a new rule rather than modify this one. It's not the sake of history, it's the sake of users

[legau]

I'm a bit confused by your argument, this PR only removes false positives so there would be no breaking change on code bases not using Black

[sigmavirus24]

People have style guides that conflict with Black and it's influence on current pep-0008. Allowing that whitespace breaks those users who expect consistency enforced by pycodestyle as new code can use either style which isn't what people keeping this enabled want. How is that difficult to understand?

[lucasmaheo]

You're misattributing why we won't change this in a way that seems trollish (deliberately wrong for the purpose of eliciting a response that does what you want).

Sorry, it truly wasn't trollish in my mind.

The reality is that many companies and open source projects have code bases not using black relying on this time e for consistency and compliance. This is why it was suggested to create a new rule rather than modify this one. It's not the sake of history, it's the sake of users

I can agree with another separate rule, but default behaviour should still be based on current pep8. I would split E203 into two rules:

• the current pep8-conforming rule on colons;
• a separate rule to enforce the check in slices as well. This additional rule could for some time still be activated by default for backwards compatibility, with warnings that it will be disabled by default at some point in the future.

I don't know if that's been done yet, and if you'd be open to that.

Just for the sake of mentioning it, and in case I misunderstood that: Black actually recommends to disable E203 altogether, leaving valuable checks behind. That's not a "working solution" for me, that's worrying.

[asottile]

should still be based on current pep8

it cannot and should not. pep8 is self contradicting and ever changing

[asottile]

That's not a "working solution" for me, that's worrying.

you shouldn't care about the suite of whitespace checks as black is managing that for you

[lucasmaheo]

OK, I guess I understand your point now. In this case, maybe you should close this PR and the corresponding issue. I'll let you get through your decision process on that.
Thanks for your comments.