Black: Should assignments manage optional parentheses automatically?

Created on 9 Jun 2018  路  10Comments  路  Source: psf/black

Original:

class Variant:
    pass

class Foo:

    @property
    def SupportedMimeTypes(self):
        mimetypes = 'audio/musepack;application/musepack;application/x-ape;' \
            'audio/ape;audio/x-ape;audio/x-musepack;application/x-musepack;' \
            'audio/x-mp3;application/x-id3;audio/mpeg;audio/x-mpeg;' \
            'audio/x-mpeg-3;audio/mpeg3;audio/mp3;audio/x-m4a;audio/mpc;' \
            'audio/x-mpc;audio/mp;audio/x-mp;application/ogg;' \
            'application/x-ogg;audio/vorbis;audio/x-vorbis;audio/ogg;' \
            'audio/x-ogg;audio/x-flac;application/x-flac;audio/flac'
        return Variant('as', mimetypes.split(';'))

Black reformats as:

class Variant:
    pass


class Foo:
    @property
    def SupportedMimeTypes(self):
        mimetypes = "audio/musepack;application/musepack;application/x-ape;" "audio/ape;audio/x-ape;audio/x-musepack;application/x-musepack;" "audio/x-mp3;application/x-id3;audio/mpeg;audio/x-mpeg;" "audio/x-mpeg-3;audio/mpeg3;audio/mp3;audio/x-m4a;audio/mpc;" "audio/x-mpc;audio/mp;audio/x-mp;application/ogg;" "application/x-ogg;audio/vorbis;audio/x-vorbis;audio/ogg;" "audio/x-ogg;audio/x-flac;application/x-flac;audio/flac"
        return Variant("as", mimetypes.split(";"))

Present on master and current release.

design parentheses question

Most helpful comment

I found a simple case that might be related to this issue.

Input:

aaaa.bbbbbbbbbb["ccccccccccccc"] = "dddddddddddddddddddddddd"

Black's output (black.now.sh, master branch, default settings):

aaaa.bbbbbbbbbb[
    "ccccccccccccc"
] = "dddddddddddddddddddddddd"

What I would expect:

aaaa.bbbbbbbbbb["ccccccccccccc"] = (
    "dddddddddddddddddddddddd"
)

All 10 comments

This needs fixing, thanks for the report!

As a workaround for now: wrap the right-hand side of the assignment in parentheses, Black will format this correctly for you and will keep it.

This issue also happens when having many variables left of the assignment operator =:

#!/usr/bin/python3

lorem, ipsum, dolor, sit, amet, consectetur, adipiscing, elit, sed, do, \
        eiusmod, tempor, incididunt, ut, labore, et, dolore, magna, aliqua, \
        In, publishing, graphic, design, placeholder, text, commonly, used, \
        demonstrate = (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28)
print(lorem)

becomes

#!/usr/bin/python3

lorem, ipsum, dolor, sit, amet, consectetur, adipiscing, elit, sed, do, eiusmod, tempor, incididunt, ut, labore, et, dolore, magna, aliqua, In, publishing, graphic, design, placeholder, text, commonly, used, demonstrate = (
    1,
    2,
    3,
    4,
    5,
    6,
    7,
    8,
    9,
    10,
    11,
    12,
    13,
    14,
    15,
    16,
    17,
    18,
    19,
    20,
    21,
    22,
    23,
    24,
    25,
    26,
    27,
    28,
)
print(lorem)

I don't know if that is the same bug or not though.

This issue also happens when having many variables left of the assignment operator =

This won't change.

I don't like fake foo-bar examples because they often hide why you'd do such a thing in real code. If you're really assigning 28 elements of a tuple to 28 names on the left, well, then don't do that.

If you are unpacking a call that returns a 28 element tuple, then you should have a talk with the designer of that function and adopt a named tuple instead. Then you don't have to unpack anything.

If you are unpacking a call that returns a 28 element tuple and there's nothing you can do about this call, then you can do it like this:

lorem, ipsum, dolor, sit, amet, consectetur, adipiscing, elit, sed, *rest = my_call()
do, eiusmod, tempor, incididunt, ut, labore, et, dolore, magna, *rest = rest
aliqua, In, publishing, graphic, design, placeholder, text, commonly, used, demonstrate, *rest = rest

But maybe you actually didn't need all the names anyway. Then just use indexing to pluck the names you needed.

Note on backslashes

Backslashes are one of the two places in the Python grammar that break significant indentation. The other one is multiline strings. You never need backslashes, they are used to force the grammar to accept breaks that would otherwise be parse errors. That makes them confusing to look at and brittle to modify. This is why Black always gets rid of them.

If you're reaching for backslashes, that's a clear signal that you can do better if you slightly refactor your code. I hope some of the examples above show you that there are many ways in which you can do it.

Real Code :tm: of the example posted by genodeftest: https://github.com/exaile/exaile/blob/5734ebb69f03812795e0280dee5262e2c6d5ad0b/plugins/grouptagger/gt_mass.py#L47

The Gtk.Child.widgets thing returns placeholders (I think it's a list, not a tuple?) that are later replaced when the composite widget is instantiated (as a form of dependency injection I suppose). The API is written this way as shorthand instead of typing GtkTemplate.Child.Widget over and over again, and is particularly useful with large numbers of widgets.

How would you design that API without requiring lots of repetitive copy/paste?

(and perhaps that issue belongs in a separate issue)

Looks like GitHub lost my comment that I put here. So a short version follows:

Before

class GtMassRename(Gtk.Window):
    ...

    found_label,    \
        playlists,      \
        replace,        \
        replace_entry,  \
        search_entry,   \
tracks_list = GtkTemplate.Child.widgets(6)

Suggestion 1

Put as many names as they fit on one line. That changed the expression from 6 lines to two.

class GtMassRename(Gtk.Window):
    ...

    found_label, playlists, replace = GtkTemplate.Child.widgets(6)
    replace_entry, search_entry, tracks_list = GtkTemplate.Child.widgets(6)

Suggestion 2

Alias a popular name that you're using many times in your file. I do that very often.

W = GtkTemplate.Child.Widget

class GtMassRename(Gtk.Window):
    ...

    found_label = W()
    playlists = W()
    replace = W()
    replace_entry = W()
    search_entry = W()
    tracks_list = W()

@ambv sorry for the example code and many thanks for the suggestions!

I thought about this some more and the problem here is that Black currently only manages optional parentheses automatically in statements, not in expressions.

I realize that assignments are statements (at least so far). However, there are other assignment-like parts of expressions (like dictionary "key: value" or keyword arguments in function calls). It would feel inconsistent to put extra parentheses in one case but not in those others.

Why not do it everywhere and be done with it? Well:

  • automatic management of those parentheses would mean Black will remove them when it decides that a different style is better; (and sometimes it would be wrong)
  • putting extra parentheses in those places would sometimes cause proliferation of brackets (esp. in the case of function calls and dictionary values), which - again - sometimes would look worse than the current situation.

Similar problem around await is in #275 and again, the worry is that this is an expression, and would complicate formattings that are simpler today.

I found a simple case that might be related to this issue.

Input:

aaaa.bbbbbbbbbb["ccccccccccccc"] = "dddddddddddddddddddddddd"

Black's output (black.now.sh, master branch, default settings):

aaaa.bbbbbbbbbb[
    "ccccccccccccc"
] = "dddddddddddddddddddddddd"

What I would expect:

aaaa.bbbbbbbbbb["ccccccccccccc"] = (
    "dddddddddddddddddddddddd"
)

I have another case, close to what was reported in #405, though I have a different preferred resolution. Note that this was indented, in a library with preferred line lengths of 80.

Here's what I initially wrote:

X = np.random.randint(0, 100, (1000, 100)) \
    * np.random.binomial(1, 0.3, (1000, 100))

Which black reformatted to:

X = np.random.randint(0, 100, (1000, 100)) * np.random.binomial(
    1, 0.3, (1000, 100)
)

Which I find hard to read, since a call is being split onto multiple lines. I agree that \ is not great, so I prefer something like:

X = (
    np.random.randint(0, 100, (1000, 100))
    * np.random.binomial(1, 0.3, (1000, 100))
)

However black reformats this the same as the original case. Something that I thought was interesting here is that black actually removes the parenthesis when formatting. I expected black to prefer (or at least not remove) the fluent-like formatting.

Here's an example that reproduces on the black playground:


Example

import numpy as np

def gen_x():
    def inner_x():
        X = (
            np.random.randint(0, 100, (1000, 100))
            * np.random.binomial(1, 0.3, (1000, 100))
        )
        return X

    return X

Was this page helpful?
0 / 5 - 0 ratings