PyCQA / pycodestyle

Simple Python style checker in one Python file
https://pycodestyle.pycqa.org
Other
5.05k stars 751 forks source link

possible false positive E303 #168

Closed dimaqq closed 8 years ago

dimaqq commented 11 years ago

Consider following valid code, at module top level:

# snip
class Foo:
    pass

class Bar:
    pass
# snip

Consider making it conditional, at module top level:

# snip
if something:
    class Foo:
        pass

    class Bar:
        pass
# snip

pep8 reports E303 too many blank lines (2) before Bar in the conditional case. semantically it is a false positive. syntactically it is an unclear case.

from PEP 8: Separate top-level function and class definitions with two blank lines. Method definitions inside a class are separated by a single blank line. Extra blank lines blah blah blah...

this particular case doesn't fall in either clearly defined category.

florentx commented 11 years ago

I agree that it is in the gray area.

I did not receive any criticism for this behavior before, and it seems acceptable to me.

However it is open for discussion.

garyd203 commented 11 years ago

An alternate point of view is that code like the above is more like a script than a standalone module (since it is not just a collection of definitions). If this distinction is meaningful, then we could treat this situation like code inside a function, and hence require a single blank line to separate logically distinct sections.

That said, this particular situation feels like it should be valid. However, that is more of a subjective judgment than something that I can describe in an algorithm.

chrismedrela commented 11 years ago

Does anybody use conditional classes? I wonder if this issue is a real problem. If not, this ticket could be closed.

garyd203 commented 11 years ago

This situation applies to top-level functions, in addition to classes.

I have seen conditional functions used as a technique to provide OS-dependent implementations. For example, the urllib module in Python 2.7 has:

if sys.platform == 'darwin':
    ...
    def proxy_bypass(host):
        ...

    def getproxies():
        ...

elif os.name == 'nt':
    ...
    def proxy_bypass(host):
        ...

    def getproxies():
        ...

Granted, there are other (perhaps better) ways to achieve this, but nonetheless it is a real technique.

jayvdb commented 9 years ago

I inherited a bunch of conditional classes, which was a OS-dependent implementation as suggested by garyd203 , and were basically this recipe:

http://stackoverflow.com/questions/878972/windows-cmd-encoding-change-causes-python-crash/3259271#3259271

The fix was not to worry about pep8's limitations, but completely overhaul it to move the classes to the top-level. https://gerrit.wikimedia.org/r/#/c/239540/3/pywikibot/userinterfaces/win32_unicode.py,cm

IanLee1521 commented 8 years ago

Closing as I feel that since it doesn't fall into either of the "two line separation" cases, this should be handled with a single line of indentation between the classes. If there is a strong opinion, I would recommend reaching out to the python-dev list to try to get the PEP-8 document updated to clarify this case.