long dotted identifiers that are correct length are flattened out into too-long lines

[zzzeek]

Operating system: fedora 29
Python version: 3.7.0
Black version: 18.9b0
Does also happen on master: yup

Sorry, it's me again from #656, another variant on Black taking code that is pep8 and making it not pep8, a series of dotted identifiers are flattened out to be too long when no change was required to keep it pep8:

dbapi = None


class FooTest:
    def test_result(self):
        self.assertEqual(
            [
                c[2]
                for c in dbapi.connect.return_value.
                cursor.return_value.execute.mock_calls
            ],
            [
                {"direct": True},
                {"direct": True},
                {"direct": True},
                {"direct": True},
                {"direct": False},
                {"direct": True},
            ],
        )

black makes it:

dbapi = None


class FooTest:
    def test_result(self):
        self.assertEqual(
            [
                c[2]
                for c in dbapi.connect.return_value.cursor.return_value.execute.mock_calls
            ],
            [
                {"direct": True},
                {"direct": True},
                {"direct": True},
                {"direct": True},
                {"direct": False},
                {"direct": True},
            ],
        )

this one I can't work around with parenthesis, I have to assign to an intermediary variable to work around which is fairly non-ideal (ill add parenthesis).

if black had options to change all this, then I'd use those, but your philosophy is to not have any options :) so...posting an issue seems more critical.

[zzzeek]

OK parens does work here, so I'll keep spitting out the parens (correction again, it doesn't work in all cases)

[zzzeek]

this is actually affecting me in several places where I have to add a "fmt off" to work around it. example:

class Thing:
    def _some_method(self):
        if self.flag:
            self.do_thing()
        else:
            for mapper in self.iterate_to_root():
                if mapper.polymorphic_on is not None:
                    if self.persist_selectable is mapper.persist_selectable:
                        self.polymorphic_on = mapper.polymorphic_on
                    else:
                        self.polymorphic_on = (
                            self.persist_selectable.corresponding_column(
                                mapper.polymorphic_on
                            )
                        )

black forces a too-long line that can't be resolved unless I change the code:

class Thing:
    def _some_method(self):
        if self.flag:
            self.do_thing()
        else:
            for mapper in self.iterate_to_root():
                if mapper.polymorphic_on is not None:
                    if self.persist_selectable is mapper.persist_selectable:
                        self.polymorphic_on = mapper.polymorphic_on
                    else:
                        self.polymorphic_on = self.persist_selectable.corresponding_column(
                            mapper.polymorphic_on
                        )

[zzzeek]

I need to make it into:

    self.polymorphic_on = (
        self.persist_selectable
    ).corresponding_column(mapper.polymorphic_on)

which looks very awkward.

[pauloalem]

@zzzeek I think this #67 is when the decision to make it behave like this was made.
I'm currently in the process of trying to migrate a large codebase to use black but the amount of column size issues introduced is too much.
I'm trying to manually fix these, but it's hard with a moving target.

[ambv]

Fluent interfaces are one thing, but producing code that is breaking the line length limit should be avoided. I do believe that the input is kind of a stretch already but the automated tool should help, not work against you.

[YodaEmbedding]

Your code is 6 indents (== 24 spaces) deep. It's called arrow code. See CodingHorror and this blog. Here's my attempt at flattening it out:

class Thing:
    def _some_method(self):
        if self.flag:
            self.do_thing()
            return

        for mapper in self.iterate_to_root():
            if mapper.polymorphic_on is None:
                continue
            if self.persist_selectable is mapper.persist_selectable:
                self.polymorphic_on = mapper.polymorphic_on
            else:
                self.polymorphic_on = self.persist_selectable.corresponding_column(
                    mapper.polymorphic_on
                )

Now it's 4 indent levels. It looks like Black is ignoring my imposed limit of 79 characters, though.

---

Ideally, I would prefer:

                # 64 characters max length
                self.polymorphic_on = (
                    self.persist_selectable
                    .corresponding_column(mapper.polymorphic_on)
                )

...instead of:

                # 83 characters max length
                self.polymorphic_on = self.persist_selectable.corresponding_column(
                    mapper.polymorphic_on
                )

A manual way of preventing Black from doing this is by introducing a new variable:

                # 71 characters max length
                modifier = self.persist_selectable.corresponding_column
                self.polymorphic_on = modifier(mapper.polymorphic_on)

...which looks nicest, though at a small cost of an extra assignment. But it's python, so who's counting those, anyways?

[zzzeek]

hoo boy, I'm dealing with a 200K line codebase and the black formatting team is going to critique the spots where it has three "if" statements nested? Yes, it is widely known that too many nested ifs/fors are an anti-pattern but you might not be aware that Python imposes a crushing performance penalty for function calls, which means in performance critical sections it is often necessary to resort to inlining. The example here, which note is not the actual code, can have one of the "ifs" flattened with the guard thing sure but often there's no way to flatten things more without resorting to multiple methods which means you lose the ability to inilne.

I don't think Black's mission is well served if it decides to balk on various formatting scenarios that are deemed unworthy. Sorry for the rant, but yes, I am quite familiar with overly nested code and that the code given is a little bit nested, however Python is not like C code, you can't always do things in the "best" way if you are dodging performance issues, and additionally, sometimes code is simply not in its final form yet, having been written in an expedient way first just to get something working. Surely throwaway code deserves to be pep8 formatted as well? I would hope the Black team agrees.

Also note I'm working within old school 79 characters which makes these scenarios all the more likely. Have a nice day!

[ambv]

@zzzeek, note that @YodaEmbedding is not a contributor and is not speaking on behalf of the project.

To paraphrase what I said above, deeply nested code and other strings of tokens which are very complex regardless of how you format them make it hard for an automated tool to help. At the same time, that tool should not make matters even worse.

That's why this issue is left open. There's a definitely improvements we can make to the tool. Sorry if anything we've said here felt condescending, that was nobody's intention.

[1oglop1]

Is there any progress on this?

I just found out that black is violating line length for long call chains and keeps changing formatted long calls so I had to use # fmt: off

class C:
    dddddddddddddddddddddddddddddddddddd = 1


class B:
    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = C


class A:
    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = B


y = {
    # fmt: off
    "ccccccccccccccccccccccccccccc": (
        A
        .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
        .bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
        .dddddddddddddddddddddddddddddddddddd
    )
    # fmt: on
}

# formatted by black
x = {
    "ccccccccccccccccccccccccccccc": A.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.dddddddddddddddddddddddddddddddddddd
}
print(x, y)

playground

If I could vote for the format I like the format from YodaEmbedding's suggestion

self.polymorphic_on = (
                    self.persist_selectable
                    .corresponding_column(mapper.polymorphic_on)
                )

[JelleZijlstra]

Duplicate of #510