yapf icon indicating copy to clipboard operation
yapf copied to clipboard

Function arg indentation changes unpredictably.

Open dseomn opened this issue 4 years ago • 8 comments

Hi,

https://github.com/dseomn/pepper-music-player/commit/23b7001d3d054b950d8f42ef48eb828ba82fc9d4 is formatted with yapf 0.28.0, google style, and it looks right to me. The CI ran with yapf 0.29.0 though, and it wanted to change the indent in a way that seems wrong to me: https://github.com/dseomn/pepper-music-player/runs/380708720

I tried coming up with a simpler case to reproduce the issue, and I've got one, but it seems unrelated to the change above, so I don't think I understand what's happening at all. Anyway, this passes 0.28.0 with google style, but fails on 0.29.0:

def f(
        a: str,
        b: str,
):
    pass

However, this passes both 0.28.0 and 0.29.0:

def f(
        a: str,
        b: str,
) -> None:
    pass

I don't understand what's going on here at all, but it seems like a bug to me.

dseomn avatar Jan 09 '20 05:01 dseomn

I'm hitting this issue in my code as well, formats correctly with 0.28.0 but incorrectly with 0.29.0.

martsa1 avatar Jan 31 '20 17:01 martsa1

I introduced a regression in these types of formatting between 0.28.0 and 0.29.0. I recently added a fix for it. Could you try HEAD to see if it's still failing for you?

bwendling avatar Jan 31 '20 20:01 bwendling

Just tested with head of master, same issue appears to still be in place I'm afraid.

Minimal repro I tested with:

from typing import List, Set


def sample_function(
        argument_one: List[str],
        argument_two: List[str],
        argument_three: List[str],
        argument_four: List[str],
) -> Set[str]:
    pass

Installing HEAD with: pip install --upgrade git+https://github.com/google/yapf.git, and running yapf --style google -d tmp.py yielded the following:

--- tmp.py      (original)
+++ tmp.py      (reformatted)
@@ -2,9 +2,9 @@


 def sample_function(
-        argument_one: List[str],
-        argument_two: List[str],
-        argument_three: List[str],
-        argument_four: List[str],
+    argument_one: List[str],
+    argument_two: List[str],
+    argument_three: List[str],
+    argument_four: List[str],
 ) -> Set[str]:
     pass

martsa1 avatar Feb 02 '20 17:02 martsa1

I'm getting the same results as @ABitMoreDepth, but looking at the style guide more, I'm not actually sure what the correct behavior is. The indentation section doesn't say anything about function arguments in a def statement. The type annotation line breaking section does have examples, but those follow Google's internal 2-space indentation style instead of the external 4-space style. @gwelymernans, do you know what the intent was there? If not, I'll file a bug and maybe fix the indentation in the examples.

dseomn avatar Feb 02 '20 22:02 dseomn

For what it's worth, HEAD seems to indent incorrectly for PEP8 now:

dseomn@solaria:~/Code/yapf (git: master %=)$ git rev-parse HEAD
e3b159be5fbac6bc2194cd1f90ec3f6f8ba60f3c
dseomn@solaria:~/Code/yapf (git: master %=)$ python3 -m yapf --style=pep8 foo.py 
def x(
    a,
    b,
):
    pass

dseomn avatar Feb 02 '20 22:02 dseomn

@gwelymernans have you had a chance to poke this?

martsa1 avatar Mar 05 '20 08:03 martsa1

any update?I got this after formating if I set column_limit=88:

class BasicMather(Model):
    def __init__(
            self,
            vocab: Vocabulary,
            embedder: TextFieldEmbedder,
            encoder: Seq2VecEncoder,
            dropout: float = None,
            namespace: str = "tokens",
            label_namespace: str = "labels",
            initializer: InitializerApplicator = InitializerApplicator(),
            **kwargs,
    ):
        pass

    def forward(
        self,
        text: TextFieldTensors,
        label: torch.Tensor = None,
    ) -> Dict[str, torch.Tensor]:
        pass

tshu-w avatar Oct 14 '20 09:10 tshu-w

Some more examples:

def test1(
    x=a,
    y=a,
):
    pass

def test2(
        x=a,
        y=a(),
):
    pass

def test3(
        x=a(),
        y=a,
):
    pass

def test4(
    x=a,
    y=a().foo,
):
    pass

def test5(
        x=a,
        y=a().foo(),
):
    pass

def test6(
        x=a,
        y=(a),
):
    pass

def test7(
    x=a,
    y=[a],
):
    pass

def test8(
    x: a() = a,
    y=a,
):
    pass

def test9(
    x=[a(), a],
    y=a,
):
    pass

Seems to be if any of the lines end in ),

.style.yapf
[style]
based_on_style=yapf
indent_width=4
column_limit=100
blank_line_before_nested_class_or_def=false
split_before_first_argument=true
split_complex_comprehension=true

KevOrr avatar Jun 08 '21 21:06 KevOrr