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.

Sign up for GitHub

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

Jump to bottom

A single conditional expression inside a list is unnecessarily parenthesized #3545

Open
yilei opened this issue Feb 4, 2023 · 5 comments · May be fixed by #4312
Open

A single conditional expression inside a list is unnecessarily parenthesized #3545

yilei opened this issue Feb 4, 2023 · 5 comments · May be fixed by #4312
Labels
F: parentheses Too many parentheses, not enough parentheses, and so on. S: accepted The changes in this design / enhancement issue have been accepted and can be implemented T: style What do we want Blackened code to look like?

Comments

@yilei
Copy link
Contributor

yilei commented Feb 4, 2023

Describe the style change

Examples in the current Black future style

items = [
    (
        {"key1": "val1", "key2": "val2", "key3": "val3"}
        if some_var == "longstring"
        else {"key": "val"}
    )
]

Desired style

items = [
    {'key1': 'val1', 'key2': 'val2', 'key3': 'val3'}
    if some_var == 'longstring'
    else {'key': 'val'}
]

Additional context

This is a change from #2278.

I think for this edge case, it shouldn't be parenthesized which also adds an extra indentation level.

Playground link: https://black.vercel.app/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4ADgAI9dAD2IimZxl1N_Wlw1a7WWyFbt4o765uThnUap6YXDKg-y2hZjBQk1tHkIJUmRf3ubZW6qh6rwYiTz123BJex-bSSddR0SO6n6vZGd9BpWqXFwR2_6FGijjM_Dne5AzdiE1FlCq8_2Dsx5LlvLchM8KOQsFr8WMFH9R1i7Ks2Es0D6bZmm7Cl6fnGsY_a6uEkAAACpTN_YLIf7gwABqwHhAQAAKlDv4rHEZ_sCAAAAAARZWg==
@yilei yilei added the T: style What do we want Blackened code to look like? label Feb 4, 2023
@ichard26 ichard26 added F: parentheses Too many parentheses, not enough parentheses, and so on. S: accepted The changes in this design / enhancement issue have been accepted and can be implemented labels Feb 5, 2023
@ichard26
Copy link
Collaborator

ichard26 commented Feb 5, 2023

I agree.

@Utkarshn10
Copy link

Why are we adding a paranthesis pair, in which case would it be useful?

@charliermarsh charliermarsh mentioned this issue Sep 3, 2023
Closed
@mariuswallraff
Copy link

I got a related case which does not use a list:

first (shorter) case:

black output for 24.2.0:

some_long_variable_name = True

abc = round(
    (
        1234.5678
        if some_long_variable_name
        else 12345.678 if not some_long_variable_name else 123456.78
    ),
    1,
)

I'd expect and prefer this:

some_long_variable_name = True

abc = round(
    1234.5678
    if some_long_variable_name
    else 12345.678 if not some_long_variable_name else 123456.78,
    1,
)

second case:

black output for 24.2.0 (with line length 99):

some_very_very_very_veeeeeeeeeeeeery_long_condition_to_check = True

abc = round(
    (
        1234.5678
        if some_very_very_very_veeeeeeeeeeeeery_long_condition_to_check
        else (
            12345.678
            if not some_very_very_very_veeeeeeeeeeeeery_long_condition_to_check
            else 123456.78
        )
    ),
    1,
)

I'd expect and prefer this:

some_very_very_very_veeeeeeeeeeeeery_long_condition_to_check = True

abc = round(
    1234.5678
    if some_very_very_very_veeeeeeeeeeeeery_long_condition_to_check
    else (
        12345.678
        if not some_very_very_very_veeeeeeeeeeeeery_long_condition_to_check
        else 123456.78
    ),
    1,
)

@cobaltt7 cobaltt7 linked a pull request Apr 17, 2024 that will close this issue
Open
cobaltt7 added a commit to cobaltt7/black that referenced this issue Apr 17, 2024
@cobaltt7
Merge branch 'main' of https://github.com/psf/black into psfgh-3545
ad570d2
@cobaltt7
Copy link
Contributor

@yilei I've opened #4312 that fixes this issue. I added it under the --unstable style (at least for now) due to #4036.

It's also worth noting that the current --unstable style already formats your example as:

items = [(
    {'key1': 'val1', 'key2': 'val2', 'key3': 'val3'}
    if some_var == 'longstring'
    else {'key': 'val'}
)]

which is better than the stable style, but could still be improved.

I took a look at the examples @mariuswallraff provided, but I believe that change was the intention of #2278 in the first place, based off the test cases it adds.

@mariuswallraff
Copy link

Thank you for the fix and for having a look at my post. After reading the other pull request and issue again, I have to agree with you, that change was intentional for the cases I posted. I'll have to get used to the extra indent. 😄

JelleZijlstra added a commit to cobaltt7/black that referenced this issue Apr 24, 2024
@JelleZijlstra
Merge branch 'main' into psfgh-3545
cbf5730
cobaltt7 added a commit to cobaltt7/black that referenced this issue Jun 7, 2024
@cobaltt7
Merge branch 'main' into psfgh-3545
8c4519f
cobaltt7 added a commit to cobaltt7/black that referenced this issue Oct 6, 2024
@cobaltt7
Merge branch 'main' into psfgh-3545
85175cd
cobaltt7 added a commit to cobaltt7/black that referenced this issue Oct 8, 2024
@cobaltt7
Merge branch 'main' into psfgh-3545
d2cad58
cobaltt7 added a commit to cobaltt7/black that referenced this issue Nov 4, 2024
@cobaltt7
Merge branch 'main' into psfgh-3545
602599e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
F: parentheses Too many parentheses, not enough parentheses, and so on. S: accepted The changes in this design / enhancement issue have been accepted and can be implemented T: style What do we want Blackened code to look like?
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Remove parenthesis around sole list items cobaltt7/black
6 participants
@yilei @Utkarshn10 @cobaltt7 @ichard26 @mariuswallraff and others