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

If statements with parallel conditions should be kept parallel #1150

Closed
jogama opened this issue Nov 11, 2019 · 4 comments
Closed

If statements with parallel conditions should be kept parallel #1150

jogama opened this issue Nov 11, 2019 · 4 comments
Labels
F: linebreak How should we split up lines? F: symmetry Fixing this would require Black to understand symmetry. R: duplicate This issue or pull request already exists T: style What do we want Blackened code to look like?

Comments

@jogama
Copy link

jogama commented Nov 11, 2019

Describe the style change A clear and concise description of how the style can be
improved.

If statements with multiple parallel conditions should be kept
parallel if they don't fit on one line. "Parallel conditions" are
conditions that only have a few characters that are different, as
bellow.

Examples in the current Black style Think of some short code snippets that show
how the current Black style is not great:

def on_segment(p_i, p_j, p_k):
    if min(p_i[0], p_j[0]) <= p_k[0] <= max(p_i[0], p_j[0]) and min(
        p_i[1], p_j[1]
    ) <= p_k[1] <= max(p_i[1], p_j[1]):
        return True
    else:
        return false

Desired style How do you think Black should format the above snippets:

def on_segment(p_i, p_j, p_k):
    if (min(p_i[0], p_j[0]) <= p_k[0] <= max(p_i[0], p_j[0])
    and min(p_i[1], p_j[1]) <= p_k[1] <= max(p_i[1], p_j[1])
    ):
        return True
    else:
        return false

Additional context Add any other context about the problem here.

Please feel free to ask any clarifying questions, or critique the desired style above.

@jogama jogama added the T: style What do we want Blackened code to look like? label Nov 11, 2019
@Jma353
Copy link
Contributor

Jma353 commented Nov 11, 2019

I don't necessarily agree with the desired style.

I think the following might be a bit better:

def on_segment(p_i, p_j, p_k):
    if (
      min(p_i[0], p_j[0]) <= p_k[0] <= max(p_i[0], p_j[0]) and 
      min(p_i[1], p_j[1]) <= p_k[1] <= max(p_i[1], p_j[1])
    ):
        return True
    else:
        return false

@jogama
Copy link
Author

jogama commented Nov 11, 2019

@Jma353 I think that looks good too.

@opk12
Copy link

opk12 commented Nov 14, 2019

axes = [0, 1]
minimums = [min(end_point_1[axis], end_point_2[axis]) for axis in axes]
maximums = [max(end_point_1[axis], end_point_2[axis]) for axis in axes]
return all(minimums[axis] <= test_point[axis] <= maximums[axis] for axis in axes)

In general, Black helps find code that is not trivial to parametrize or extend (adding a third axis and excluding the segment endpoints should not be multi-line changes) or not trivial to debug (lack of intermediate variables to print).

Black cannot change readability (as in being "correct by design", pythonic, natural-language-looking), it decreases the eye movements needed to parse.

@JelleZijlstra JelleZijlstra added F: linebreak How should we split up lines? F: symmetry Fixing this would require Black to understand symmetry. labels May 30, 2021
@felix-hilden
Copy link
Collaborator

I think this is yet another case of #2156 and @Jma353's desired style is the one proposed there, so I'll close this issue in favor of it!

@felix-hilden felix-hilden added the R: duplicate This issue or pull request already exists label Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: linebreak How should we split up lines? F: symmetry Fixing this would require Black to understand symmetry. R: duplicate This issue or pull request already exists T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

5 participants