E303: Does not report, that a blank line between classes is needed, if they're defined inside an if statement

[jgandert]

As stated in PEP8:

Separate top-level function and class definitions with two blank lines.

if __name__ == '__main__':
    class Pos:

        def __init__(self, x, y):
            self.x = x
            self.y = y

    class Lifetime:

        def __init__(self, lifetime):
            if lifetime < 0:
                raise ValueError('lifetime must be above 0')
            self.lifetime = lifetime

    class Name:

        def __init__(self, name):
            self.name = '"' + name + '"'

[sigmavirus24]

That will define the bindings Pos, Lifetime, and Name as globals. I'm not sure if I agree that they should be separated by two blank lines. Maybe @ncoghlan can help us out here. I can't decide if top-level is referring to indentation or scope.

[jgandert]

@sigmavirus24 Hmm, I wonder about that as well.

Whatever the PEP8 actually means, the class statement hanging there between two methods definitely looks a bit off to me:

            self.y = y

    class Lifetime:

        def __init__(self, lifetime):

Even without the blank line after the class statement, I would prefer two blank lines between classes, because there's one line between methods of the class:

    class Lifetime:
        def __init__(self, lifetime):
            if lifetime < 0:
                raise ValueError('lifetime must be above 0')
            self.lifetime = lifetime

        def do_stuff(self):
            pass


    class Name:
        def __init__(self, name):
            self.name = '"' + name + '"'

[sigmavirus24]

So that's fine if that's your preference. Does the latter raise a warning or error from pep8 though? If not, just go ahead and do it until we can clear up what the document is actually suggesting. I personally don't feel too strongly about this matter so either outcome is fine by me. I just want to get it right and not add additional checks that will upset developers.

[jgandert]

For this

if True:
    class Lifetime:
        def __init__(self, lifetime):
            if lifetime < 0:
                raise ValueError('lifetime must be above 0')
            self.lifetime = lifetime

        def do_stuff(self):
            pass


    class Name:
        def __init__(self, name):
            self.name = '"' + name + '"'

pep8 returns:

bla.py:12:5: E303 too many blank lines (2)

[IanLee1521]

I would tend to think that the document is referring to "top-level" as an indentation, and therefore this previous example is "global" but not "top-level". I agree this is something that could be clarified in the document.

[jgandert]

@IanLee1521 I'm no authority on this nor am I criticising you, but I don't think this makes sense (and it looks weird with one blank line).

How do we know if it is referring to indentation and not scope? And what could the logic behind "only one blank line between classes within an if statement" be? (honest question)

[ncoghlan]

For cases like the one described here, "do whatever seems most readable in the specific situation" is about as good as the PEP can do - there are too many variables affecting readability once you move the opening header line for class and function definitions away from the left hand margin.

In particular, the PEP guidelines on vertical whitespace are designed to create coherency within class and function definitions by creating additional space between them. That's desirable at the module top level, but when a suite is involved, excessive use of vertical whitespace means you end up reducing the coherency of the suite.

So once you get beyond unindented code, the overarching guidelines on when to ignore other PEP guidelines are far more likely to come into play. In particular, this one: "[Don't apply PEP 8 guidelines when] ... applying the guideline would make the code less readable, even for someone who is used to reading code that follows this PEP".

[ncoghlan]

From a pep8 perspective, I think "no opinion" is the right approach here - folks that want a tool that also covers cases where PEP 8 defers to the developer's judgement would likely be better served by starting with pylint's expansive default settings, and tuning those down to a subset they like.

[jgandert]

@ncoghlan Hmm, do I get it right that by "no opinion" you mean, that pep8 shouldn't return bla.py:12:5: E303 too many blank lines (2)? That would be great, because it would solve my problem more or less.

[ncoghlan]

Yes, I think if a function or class definition is nested inside another suite, pep8 would be better off ignoring the vertical whitespace guidelines. I don't know how practical that is to implement, though.

[sigmavirus24]

I'll let @IanLee1521 comment on the feasibility