Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Nested function calls confuse E128 #146

Closed
zaneb opened this Issue Dec 21, 2012 · 5 comments

Comments

Projects
None yet
3 participants

zaneb commented Dec 21, 2012

pep8 1.3.4 fails to detect an E128 error in a nested function call if subsequent lines are outdented to match the function call.

Tested with the following example file:


def correct():
    return a_really_really_long_function_call(the_first_parameter,
                                              another_function_call(
                                                  'string',
                                                  other_func_param=param_val),
                                              param3=third_func(some_arg))


def fails_e128():
    return a_really_really_long_function_call(the_first_parameter,
                                              another_function_call('string',
                                                  other_func_param=param_val),
                                              param3=third_func(some_arg))


def should_fail():
    # Actually passes
    return a_really_really_long_function_call(the_first_parameter,
                                              another_function_call('string',
                                              other_func_param=param_val),
                                              param3=third_func(some_arg))

Output:

$ pep8 --version
1.3.4
$ pep8 pep8_test.py 
pep8_test.py:14:51: E128 continuation line under-indented for visual indent
Contributor

florentx commented Dec 21, 2012

Thank you for the feedback.

The E121-E128 codes are new checkers added in version 1.2.
The code is already quite complex to have a good balance between freedom and useful conventions.
I don't believe it is necessary to make it more strict.

However if some patch is proposed, it could be considered for inclusion.

zaneb commented Feb 27, 2013

Fair enough.

For the record, I actually think it's too strict. The second example above is actually quite readable, though not perfect. But because pep8 is trying to be too clever for its own good it rejects this and encourages people to change it to the third example, which is clearly worse in every possible way. This doesn't seem like a win, and I can't help thinking that the (Python) world might be a better place if these checks were disabled by default.

Contributor

florentx commented Feb 27, 2013

Indeed, I misinterpreted the bug report thinking that you want to make it more strict :-)

However, I believe that the checker is right when it rejects the 2nd example above fails_e128().
Because the PEP-8 convention explicitly prohibits this:

"When using a hanging indent the following considerations should be applied; there should be no arguments on the first line"

In addition the following constructs are accepted too:

def hanging_then_visual():
    return a_really_really_long_function_call(
        the_first_parameter,
        another_function_call('string',
                              other_func_param=param_val),
        param3=third_func(some_arg))
def hanging_then_hanging():
    return a_really_really_long_function_call(
        the_first_parameter,
        another_function_call(
            'string',
            other_func_param=param_val),
        param3=third_func(some_arg))

For more information on the rationale for the E12 checks, you can read the discussion on the feature request #64.

zaneb commented Feb 28, 2013

Yeah, I was just going to leave the test case there rather than be that guy - you know, the one who insists on relitigating long-settled, well-documented design decisions in a bug report. But I changed my mind because I figure it is useful to report how this stuff is being used in the real world ;) (I actually found some code that had been "cleaned up" like this to satisfy pep8.)

Anyway, this feature is currently occupying the "simple but strict" quadrant - the unhappy medium between "simple but permissive" and "smart and strict", either of which would be an improvement, though I would personally pick the former. (Sadly, nobody has yet invented the smart and permissive linter that I actually want, for any language, and I doubt anyone ever will).

Owner

IanLee1521 commented Jun 2, 2016

Closing this issue as it seems to have been resolved.

FWIW -- I personally would do:

def my_function():
    return a_really_really_long_function_call(
        the_first_parameter,
        another_function_call('string', other_func_param=param_val),
        param3=third_func(some_arg)
    )

Which dodges the line limit, or just moving straight to moving computing the inner function into it's own line before the computation:

def my_function():
    param2 = a_very_long_function_call('string', other_func_param=param_val),

    return a_really_really_really_really_really_really_long_function_call(
        the_first_parameter,
        param2,
        param3=third_func(some_arg)
    )

@IanLee1521 IanLee1521 closed this Jun 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment