Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Spurious E301 for nested function definitions #28

Open
walkerh opened this Issue · 7 comments

7 participants

@walkerh

Simple nested functions without preceeding blank line are not a violation of PEP 8. It is up to the human to decide readability in this case.

pep8-0.6.1 will emit errors in cases where an extra blank line is not required or appropriate.

Example code, pep8_bug.py:

"""Demonstrate bug"""


def safe_accessors(dictionary=False):
    """simplified from complex test code..."""
    if dictionary:
        _setattr, _getattr, _delattr = setitem, getitem, delitem
        def _hasattr(_dict, value):
            return value in _dict
    else:
        _setattr, _getattr, _delattr, _hasattr = (
            setattr, getattr, delattr, hasattr
        )
    return _setattr, _getattr, _delattr, _hasattr

pep8 output:

pep8_bug.py:9:9: E301 expected 1 blank line, found 0

The correct output is to be silent (or at most emit a silence-able warning).

Reference: "PEP 8" / "Code lay-out" / "Blank Lines"

@JensRantil

I am too experiencing this with pep8 1.3.3.

@JensRantil

Without looking at the code, I guess the secret would be to look at the scope of the defined function. If it is in the scope of another def, no error or warning should be emitted.

@JensRantil JensRantil referenced this issue from a commit in JensRantil/pep8
@JensRantil JensRantil Minimal test case for issue #28
Currently there are false negatives when nesting python functions.
82c3a9e
@attilaolah attilaolah referenced this issue from a commit in attilaolah/pep8
@attilaolah attilaolah a simple fix for nested functions (fixes #28) f0d4740
@attilaolah attilaolah referenced this issue
Closed

Fix #28 #219

@attilaolah

I've attached a pull request that fixes this and makes the test added in 82c3a9e pass.

I'm not sure though what to do with classes that are nested inside functions, e.g.

def foo():
    class Test(object):
        def method_1(self):
            pass
        def method_2(self):
            pass

Should method_1 and method_2 be separated by blank lines because they are methods, even though they are define in a closure? (If yes, then I need modifications to my pull request.)

@chrismedrela

I'd like to cover much broader set of cases. What about using a whitelist instead of blacklist? We can assume that every class definition which is indented and every function definition which is indented more than 2 levels are nested definitions. This covers all cases except for a function nested in a function. This case can be covered by a variation of @attilaolah solution -- test if there is any function definition before, but stop when you find a class definition.

@chrismedrela

What about my PR #232?

@chrismedrela chrismedrela referenced this issue from a commit in chrismedrela/pep8
Christopher Medrela Fixed #28 -- relaxed E301 for nested definitions e5c0576
@gward

I agree with @chrismedrela: nested classes should be considered similarly to nested functions, on the grounds that PEP 8 says what to do about top-level classes and functions, and says nothing about nested functions or classes. If pep8 is going to be neutral about nested functions, then it should be equally neutral about nested classes.

@florentx
Collaborator

PEP 8 says:

Method definitions inside a class are separated by a single blank line.

I would prefer to keep the checks for these nested classes with some new error code, which could be ignored in the configuration.

For example:

E311 expected 1 blank line, found 0 (nested definition)
@scopatz scopatz referenced this issue in pyne/pyne
Merged

PyNE in a State of Decay #614

@IanLee1521 IanLee1521 self-assigned this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.