classification
Title: Update PEP-8 regarding binary operators
Type: enhancement Stage: patch review
Components: Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: IanLee1521, Martín Gaitán, Rosuav, barry, berker.peksag, gvanrossum, methane
Priority: normal Keywords: patch

Created on 2016-04-15 07:06 by IanLee1521, last changed 2016-04-15 19:24 by Rosuav. This issue is now closed.

Files
File name Uploaded Description Edit
wrap-before-binary-operator.patch IanLee1521, 2016-04-15 07:05 Update PEP-8 to specify wrapping before rather than after a binary operator
Messages (17)
msg263453 - (view) Author: Ian Lee (IanLee1521) * Date: 2016-04-15 07:05
Following up from discussion on python-ideas [1] about updating PEP-8 regarding wrapping lines before rather than after binary operators.
msg263454 - (view) Author: Ian Lee (IanLee1521) * Date: 2016-04-15 07:09
Discussion link missing from msg263453: https://mail.python.org/pipermail/python-ideas/2016-April/039774.html
msg263455 - (view) Author: Ian Lee (IanLee1521) * Date: 2016-04-15 07:23
Link to GitHub branch with the patch as a commit [1].

[1] issue26763">https://github.com/python/peps/compare/master...IanLee1521:issue26763
msg263489 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-04-15 13:05
issue26763">https://github.com/python/peps/compare/master...IanLee1521:issue26763
msg263490 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-04-15 13:07
Roundup doesn't link to Github's branch comparing URL correctly.
How about just create pull request on Github?
msg263495 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-04-15 13:33
Please don't create a pull request on GitHub (python/peps is read-only). Attaching wrap-before-binary-operator.patch is enough.
msg263497 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-04-15 13:52
How about recommend using parentheses to avoid different level operators have same indent level?

ok:
            if (width == 0
                and height == 0
                and color == 'red'
                and emphasis == 'strong'
                or highlight > 100
            ):
                ...

better:
            if ((width == 0
                 and height == 0
                 and color == 'red'
                 and emphasis == 'strong')
                or highlight > 100
            ):
msg263498 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-15 14:31
I don't like the way "):" is indented in that example. I propose:

            if ((width == 0
                 and height == 0
                 and color == 'red'
                 and emphasis == 'strong')
                or highlight > 100):

Aren't there many other examples in the PEP that need to be adjusted, since we're changing the style not just for 'and' and 'or' but for all binary operators?
msg263499 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-15 14:33
I also think the PEP needs some language about the previous style being still acceptable as long as it's being used consistently (Knuth notwithstanding), and an acceptable compromise style where and/or go at the start of the line but other binary operators stay at the end.
msg263501 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-04-15 15:50
On Fri, Apr 15, 2016 at 11:31 PM, Guido van Rossum <report@bugs.python.org>
wrote:

>
> Guido van Rossum added the comment:
>
> I don't like the way "):" is indented in that example.

Me too.
I haven't know about this PEP8 update.
https://github.com/python/peps/commit/843b7d2cdb954dc0aa5ea70f140db2bd42943e1f

> I propose:
>
>             if ((width == 0
>                  and height == 0
>                  and color == 'red'
>                  and emphasis == 'strong')
>                 or highlight > 100):
>
>
Looks better to me.  But how about this?

if (width == 0
        and height == 0
        and color == 'red'
        and emphasis == 'strong'
    or highlight > 100):

Changing indentation level based on operator precedence is acceptable?
msg263506 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-15 16:13
> Changing indentation level based on operator precedence is acceptable?

I think that's a matter of personal taste and I don't think the PEP needs to have an opinion on it.
msg263508 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-15 16:46
I've updated the PEP with some more permissive language, mentioning Knuth's recommendation but allowing the old style because it's been recommended (and followed, and enforced) for decades. I've intentionally not updated other examples in the PEP to follow suit.
msg263509 - (view) Author: Ian Lee (IanLee1521) * Date: 2016-04-15 17:32
> Aren't there many other examples in the PEP that need to be adjusted, since we're changing the style not just for 'and' and 'or' but for all binary operators?

Admittedly I'd only looked in that one section last night, but I just went through the PEP and didn't see any other when I just went through the entire PEP now...

> I also think the PEP needs some language about the previous style being still acceptable as long as it's being used consistently (Knuth notwithstanding), and an acceptable compromise style where and/or go at the start of the line but other binary operators stay at the end.

I'd updated my branch on GitHub [1] but Guido, you beat me to the update while I was typing this message!

> I don't like the way "):" is indented in that example.

Honestly I don't either, I just like it more than artificially pushing the other lines forward a space. Personally, I'm not a fan of the current change either which puts the raise in line with the condition logic for the `if`::

        if (width == 0
            and height == 0
            and color == 'red'
            and emphasis == 'strong'
            or highlight > 100):
            raise ValueError("sorry, you lose")


My preference is to actually break that logic up and avoid the wrapping in the first place, as in [2]. Which in this particular class has the side benefit of that value being used again in the same function anyways.

I'm starting to realize that Brandon Rhodes really had a big impact on my ideas of styling as I've been learning Python these past few years, as this was another one style I'm stealing from that same talk [3].

> I've updated the PEP...

Great, thanks! I've created a ticket in pycodestyle [4] to update the style checker, which in turn will fix the flake8 error downstream once I get around (hopefully before PyCon) to releasing pycodestyle 1.8.0

[1] issue26763">https://github.com/python/peps/compare/master...IanLee1521:issue26763
[2] https://github.com/python/peps/commit/0c790e7b721bd13ad12ab9e6f6206836f398f9c4
[3] http://rhodesmill.org/brandon/slides/2012-11-pyconca/#naming-intermediate-values
[4] https://github.com/PyCQA/pycodestyle/issues/498
msg263510 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-15 17:43
I propose to keep the somewhat questionable examples in the PEP as an
illustration of the idea that style is debatable and cannot be reduced to
fixed rules.
msg263518 - (view) Author: Martín Gaitán (Martín Gaitán) Date: 2016-04-15 19:15
There is a typo on "reseach" instead of "research"
msg263519 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-15 19:20
OK, maybe @chrisa can fix the typo.
msg263520 - (view) Author: Chris Angelico (Rosuav) * Date: 2016-04-15 19:24
Typo fixed in peps repo.
History
Date User Action Args
2016-04-15 19:24:01Rosuavsetmessages: + msg263520
2016-04-15 19:20:25gvanrossumsetmessages: + msg263519
2016-04-15 19:15:17Martín Gaitánsetnosy: + Martín Gaitán
messages: + msg263518
2016-04-15 18:29:53barrysetnosy: + barry
2016-04-15 17:43:11gvanrossumsetmessages: + msg263510
2016-04-15 17:32:02IanLee1521setmessages: + msg263509
2016-04-15 16:46:46gvanrossumsetstatus: open -> closed
resolution: fixed
messages: + msg263508
2016-04-15 16:13:26gvanrossumsetmessages: + msg263506
2016-04-15 15:50:34methanesetmessages: + msg263501
2016-04-15 14:33:11gvanrossumsetmessages: + msg263499
2016-04-15 14:31:50gvanrossumsetmessages: + msg263498
2016-04-15 13:52:39methanesetmessages: + msg263497
2016-04-15 13:33:00berker.peksagsetnosy: + berker.peksag

messages: + msg263495
stage: patch review
2016-04-15 13:07:54methanesetmessages: + msg263490
2016-04-15 13:05:07methanesetnosy: + methane
messages: + msg263489
2016-04-15 07:35:44Rosuavsetnosy: + Rosuav
2016-04-15 07:23:58IanLee1521setmessages: + msg263455
2016-04-15 07:09:45IanLee1521setmessages: + msg263454
2016-04-15 07:06:00IanLee1521create