Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEP 8 Inspection over-generalizes blank lines around functions #457

Open
yoleg opened this issue Oct 30, 2015 · 3 comments
Open

PEP 8 Inspection over-generalizes blank lines around functions #457

yoleg opened this issue Oct 30, 2015 · 3 comments

Comments

@yoleg
Copy link

yoleg commented Oct 30, 2015

PEP 8 only requires blank lines around "top-level function and class definitions" and "method definitions inside a class". However, pep8 marks any function without a line above it as failing the rule.

This is especially troublesome for functions that are inside other functions or methods, because they are often short and have no reason for a line above:

def test():
    def success_callback():  # no need for blank line above
        print('Success! A wonderful thing has completed!')
    def error_callback():  # no need for blank line above
        print('Error! The entire program is crashing around us!')
    run_async_function(success_callback, error_callback)
@sigmavirus24
Copy link
Member

This is especially troublesome for functions that are inside other functions or methods, because they are often [in my experience] short and have no reason for a line above:

I added a clarifying phrase to your statement. If you ever venture into the OpenStack code bases there are nested functions which are anything but short. This rule helps that code base become more legible.

That said, the literal phrasing of the PEP (not its spirit which is not something I'm going to debate with anyone) means that this rule over steps its responsibilities. I'd be in favor of retaining the check but in a separate error code specifically for nested functions (if we can even easily make that distinction) such that you could ignore that code for your entire code base. Would that be a reasonable accommodation to you @yoleg?

@yoleg
Copy link
Author

yoleg commented Nov 7, 2015

@sigmavirus24 Yes, that should work. Your clarifying phrase is correct. I myself write long nested functions and would appreciate the ability to turn this inspection on and off. It is only when using callback-based libraries that I find myself using dozens of very short closure functions.

Side note 1: Perhaps this error is more closely associated with the PEP 8 rule "Use blank lines in functions, sparingly, to indicate logical sections." After all, (long) nested functions could be considered "logical sections".

Side note 2: The inspection only seems to notice blank lines missing above the nested function. It seems to me more logical to check for a blank line both above and below, as this looks much cleaner when separating long nested functions.

Side note 3: It is actually my IDE (IntelliJ with Python/ PyCharm plugin) that is using pep8 in their inspections. It lets me easily disable/ enable specific PEP8 errors.

@sigmavirus24
Copy link
Member

Re side note 2: See #400 as a related issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants