Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Space inserted after function call in slice #311

Closed
iandanforth opened this issue Jun 7, 2018 · 4 comments
Closed

Space inserted after function call in slice #311

iandanforth opened this issue Jun 7, 2018 · 4 comments

Comments

@iandanforth
Copy link

iandanforth commented Jun 7, 2018

Operating system: OSX
Python version: 3.6.4
Black version: 18.6b1
Does also happen on master: Yes

Concern

Black appears to special case function calls in slices, inserting a space after them.

a = b[foo():]

becomes

a = b[foo() :]

PEP-8 (to my reading) only specifies equal spacing around colons and does not special case function calls. The example given in PEP-8 which includes function calls happens to display that equal spacing as a single space, but no negative example is given which would forbid having consistent zero space slices. i.e.

ham[: upper_fn(x) : step_fn(x)], ham[:: step_fn(x)]

does not preclude

ham[:upper_fn(x):step_fn(x)], ham[::step_fn(x)]

Suggestion

Since it is syntactically valid, does not interact with operator precedence, conforms to previous lint standards and follows the equality requirement of PEP-8 do not insert a space in this case.

@ambv
Copy link
Collaborator

ambv commented Jun 7, 2018

Thanks for your feedback! I disagree.

PEP 8 would be endlessly long if it wanted to list an exhaustive list of allowed/disallowed. Therefore it tries to summarize the most information in the minimal set of examples.

You can see in PEP 8 that there's:

# yes
ham[lower+offset : upper+offset]
# no
ham[lower + offset:upper + offset]

This suggests that in presence of a non-trivial operand, there should be spaces around colons. A call is also treated as non-trivial (it is using the parens as an operator that triggers __call__() after all):

# yes
ham[:: step_fn(x)]

As you can see there is just a single call and there is a space between it and the colon. The fact that there's two colons is irrelevant, as I said above, PEP 8 necessarily minimizes the number of examples to show the maximum number of cases.

@iandanforth
Copy link
Author

iandanforth commented Jun 7, 2018

In the case where there is no explicit injunction in PEP-8 I think its agreed there is room for interpretation.

In this case the disagreement seems to be about whether a function call is part of the set described by "However, in a slice the colon acts like a binary operator, and should have equal amounts on either side (treating it as the operator with the lowest priority). " [emphasis mine]

In the next section the set of binary operators is further enumerated:

"Always surround these binary operators with a single space on either side: assignment (=), augmented assignment (+=, -= etc.), comparisons (==, <, >, !=, <>, <=, >=, in, not in, is, is not), Booleans (and, or, not)."

The use of () as an operator is not in this list of binary operators.

Taken together the I believe the intent of this section is not about 'non-trivial operators' but 'binary operators which have varying precedence.'

Again because the established precedent disagrees with the current behavior and the above reading of the intent and specifics of PEP-8 I respectfully reaffirm my request that this behavior be changed.

@ambv
Copy link
Collaborator

ambv commented Jun 7, 2018

The use of () as an operator is not in this list of binary operators.

The colon is acting as a binary operator. The call parentheses act as a unary operator. But we're splitting hairs here. There is an explicit "yes" example in the form of ham[:: step_fn(x)]. That's enough for established precedent for me.

Aside from PEP 8, Black in general doesn't put much weight in "established precedent". Just see:

  • always dedenting closing brackets on a new line;
  • double quotes in strings;
  • putting binary operators on the front of the line;
  • removing backslashes from the codebase.

The point of the project is to formalize and enforce a new consistent style which is a subset of PEP 8.

@iandanforth
Copy link
Author

Well it's your call, but foo() is not parsed as a unary operator. A call expression is a distinct thing. I also disagree that a positive example of a slice with consistent single spacing is the same thing as sufficient justification to exclude consistent spacing without spaces. shrug I've said my bit.

mambelli added a commit to mambelli/decisionengine that referenced this issue Oct 25, 2021
mambelli added a commit to mambelli/decisionengine_modules that referenced this issue Oct 25, 2021
mambelli added a commit to HEPCloud/decisionengine_modules that referenced this issue Oct 25, 2021
mambelli added a commit to HEPCloud/decisionengine that referenced this issue Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants