E225 should not require whitespace surrounding higher-precedence operators #166

Closed
eZanmoto opened this Issue Feb 14, 2013 · 6 comments

Comments

Projects
None yet
2 participants

At the moment, pep8 will signal an error for all of the following (defined in PEP8 as examples that should be allowed):

x = x*2 - 1
hypot2 = x*x + y*y
c = (a+b) * (a-b)

This behaviour conflicts with the following convention preceding these lines in PEP8:

If operators with different priorities are used, consider adding whitespace around the
operators with the lowest priority(ies). Use your own judgement; however, never use more
than one space, and always have the same amount of whitespace on both sides of a
binary operator.

If coding this rule is too complex for immediate implementation, I would suggest loosening the rule for the time being, so that the rule simply makes sure that there are is at most 1 space between operators and their operands.

Contributor

florentx commented Feb 15, 2013

This is a duplicate of issue #96, released with version 1.4

@florentx florentx closed this Feb 15, 2013

I must have been using an older version of the binary, however, the fix still does not allow an absence of whitespace around all higher-precedence operators in an expression, the rule seems to just be a context-unaware "allow optional spacing around **, *, / and //". However, according to the PEP8 specification of the rule, all operators at a higher precedence in a mixed precedence expression should be allowed optional spacing:

If operators with different priorities are used, consider adding whitespace around the
operators with the lowest priority(ies). Use your own judgement; however, never use more
than one space, and always have the same amount of whitespace on both sides of a
binary operator.

This should also mean that all operators inside parentheses may omit whitespace. I currently have an issue where the following statement is signalling E225 (missing whitespace around operator):

return (byte>>bit) & 1

I believe that this format for the expression should be allowed, as I have omitted the spacing for the higher precedence expression (that inside parentheses), and including the spacing for the lower precedence expression.

Contributor

florentx commented Feb 23, 2013

I confirm that since pep8 1.4, including 1.4.3, these operators accept optional whitespace (and report E226 which is ignored by default): ** * // / + -

The whitespace is still mandatory (emitting E225) around:

  • the comparison operators
  • the assignment and augmented assignment operators
  • some binary operators (bitwise, shifting and modulo): % ^ & | << >>

The reason for this choice is that all these operators bind less tightly than the arithmetic operators (except the modulo %).
The logic for the modulo % is that it is commonly used for string substitutions, and in this use case it makes sense to require spaces around it for readability.

IMHO in this example the parenthesis is enough to signal that the operation binds more tightly:

def f(byte, bit):
    return (byte >> bit) & 1

I agree that the PEP-8 specification give even more freedom regarding the whitespace around these non-arithmetic operators, however the current logic of the pep8 tool is defensible.

I re-open the issue in case someone else wants to comment on this request.
A possible solution might be to introduce again a new error code E227 which might be configured per project.

@florentx florentx reopened this Feb 23, 2013

The reason for this choice is that all these operators bind less tightly than the arithmetic operators (except the
modulo %).

While they are equal in the fact that they bind less tightly than arithmetic operators, they still have different levels of precedence relative to each other, and though the examples on PEP8 only alude to arithmetic operators, the actual wording of the rule imposes such restrictions. As such, I would imagine that the following would be allowed:

a&b | c&d
(a|b) & (c|d)

Also, consider the following snippet I presented earlier:

return (byte>>bit) & 1

The reason I would advocate this style over having whitespace surrounding the >> operator is that the PEP8 specifically encourages omitting whitespace around lower-precedence operators in parentheses (the following is titled "Yes"):

c = (a+b) * (a-b)

Over the inclusion of whitespace (the following is titled "No"):

c = (a + b) * (a - b)

While I personally prefer the style of simply surrounding all binary operators with a single space, I follow the idea that one should follow an idiomatic style if one is presented, and such is why I'm pushing this issue.

florentx added a commit that referenced this issue Feb 24, 2013

Contributor

florentx commented Feb 24, 2013

This should give enough flexibility to ignore the verification for E227 for example.

Contributor

florentx commented Feb 24, 2013

released with 1.4.4

@florentx florentx closed this Feb 24, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment