Yapf: Formatting regression

[sbarzowski]

My style.yapf:

[style]
based_on_style = pep8
DEDENT_CLOSING_BRACKETS = true

I had a code like this:

def a():
    class B:
        def c(self):
            self.fields['lorem'].label = (
                'lorem ipsum dolor sit amet blah blah x'
            )

and yapf seemed to like it, however after recent upgrade from 0.16.1 to 0.16.2 it started reformatting it to:

def a():
    class B:
        def c(self):
            self.fields['lorem'
                        ].label = ('lorem ipsum dolor sit amet blah blah x')

It seems like a terrible place for a newline. Any idea why it happens?

[jouve]

It looks related to 77ca21d9

[joelnb]

Agreed that it looks like that commit is the issue but I'm not familiar enough with the code to fix it. I'll get reading though because currently I am unable to use the latest YAPF due to this issue.

EDIT: I think that the issue is actually with 0f44f54685bdc4a2c7815a557ed469741c339937 & I will prepare a PR shortly.

[joelnb]

I made a PR but (as I said there) not convinced it is the correct approach so would be interested in the opinions of others affected by this issue.

[cybojenix]

in #419, you mentioned that #436 fixed your issue here.
I'm still experiencing the same bug referenced here.

When changing the column limit to 120, YAPF begins splitting the square brackets.

expected:

...
            translated_data['date_required'] = date_to_rfc3339(
                parse_date(translated_data['date_required'])
            )
...

output:

...
            translated_data['date_required'
                            ] = date_to_rfc3339(parse_date(translated_data['date_required']))

...

style config:

[style]
based_on_style = pep8
DEDENT_CLOSING_BRACKETS = true
COLUMN_LIMIT = 120

[cybojenix]

Quick update, I tried with YAPF 0.16.1, and I still experience the above issue

[mikelambert]

@cybojenix , the fix is here: https://github.com/google/yapf/commit/8a3b71f86b5c95b19629a778428a613881b03ca0 . And the fix says it is only in master and 0.18. According to pypi https://pypi.python.org/pypi/yapf, 0.18 was released on Sept 18th. Curious why you're using an older version 0.16.1 released in March?

[cybojenix]

@mikelambert, sorry I didn't mention. The first comment was with 0.18.0. I tried 0.16.1 afterwards since it was mentioned that 0.16.2 introduced the regression.

[cybojenix]

@mikelambert actually this looks like a different fix to the ones discussed above. Thank you for pointing this commit out, I'll try the knob later on

[mikelambert]

Oh crap, you're right, I'm totally confusing my fixes here. The fix I referenced is for a different dict-value formatting bug, unrelated to this issue. Sorry for the noise. :(

[cybojenix]

ALLOW_SPLIT_BEFORE_DICT_VALUE does not fix. It looks to be more geared towards defining a dictionary literal.

I think there should perhaps be a knob for controlling when to split inside the subscript operator (self[key]).

In my opinion, when setting, it should never be split, however it could be more acceptable to allow splitting when doing a get.

this_is_a_long_dict_name['very_long_key_name'] = some_func(
    long, list, of, args
)
this_is_a_long_variable_name = this_is_a_long_dict_name[
    'very_long_key_name'
]

not fantastic examples, but hopefully you get the picture.

[gwelymernans]

We try never to split inside a subscript unless there's absolutely no other choice. As for your examples, it would probably be able to handle them...I think. :-)

The fix someone mentioned above is in top-of-tree. It wasn't back ported to the 0.16 branch though. Are you able to use the current release?

python -m yapf --style '{based_on_style: pep8, DEDENT_CLOSING_BRACKETS: True}' /tmp/x.py
class _():
    def _():
        if True:
            translated_data['date_required'] = date_to_rfc3339(
                parse_date(translated_data['date_required'])
            )

[cybojenix]

@gwelymernans as mentioned above, this was tried on the current release (0.18.0). The reason for testing on 0.16 was to see if it was a regression introduced in 0.16.2, also mentioned above.

With the collumn limit set the 79 (inherited from pep8), the lines do dedent correctly. However with it set to 120, the subscript is split (as shown above).

[paradoxxxzero]

I'm still seeing this behavior in yapf 0.21.0 and master. I've seen new options being added but none fixed the problem:

#  python -m yapf --style '{based_on_style: pep8, dedent_closing_brackets: True, allow_split_before_dict_value: False, split_before_closing_bracket: False}'  exemple.py
def a():
    class B:
        def c(self):
            self.fields['lorem'
                        ].label = ('lorem ipsum dolor sit amet blah blah x')

with exemple.py containing what I want:

def a():
    class B:
        def c(self):
            self.fields['lorem'].label = (
                'lorem ipsum dolor sit amet blah blah x'
            )

Am I missing something?