PyCQA / pycodestyle

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

E402 doesn't respect version strings #394

Closed lewisc closed 7 years ago

lewisc commented 9 years ago
# some header comment 1
# some header comment 2
# some header comment 3
"DocString"

__version__ = "3.1"

import os

def main():
    print os.getenv('NOTREAL', 0)

if __name__ == '__main__':
    main()

pep8 returns testblah.py:8:1: E402 module level import not at top of file

When this is acceptable by PEP8's standard on version strings.

https://www.python.org/dev/peps/pep-0008/#version-bookkeeping

pep8 --version 1.6.2

lewisc commented 9 years ago

This may be incorrect, the PEP8 is unclear.

Both version bookkeeping and imports stipulate "right after the docstring," in the case of version-bookkeeping this is qualified as "before any any other code," in the case of import it is qualified as "before any module globals or constants."

I will try to contact the relevant individuals and see which is correct.

jayvdb commented 9 years ago

If this rule, preventing module metadata (__foo__ constants) occurring imports, is deemed necessary, I would appreciate if it could be given a separate code. __foo__ constants above or below imports is (almost always) purely a style decision, whereas 'delayed imports' can be very difficult to fix if they exist to prevent cyclic imports.

As mentioned on issue #351 , pywikibot is slowly fixing the 'delayed imports' problems. We would be more likely to change style re __foo__ constants to conform to pep8, if the pep8 checker could enforce it. However we cant enforce E402 while it is also used for 'delayed imports', as that will take a long time to redesign our import hierarchy.

Fwiw, the pywikibot task tracking this problem is https://phabricator.wikimedia.org/T87409

warsaw commented 9 years ago

I just ran into this in a file that included __author__ assignment between the module docstring and the first import. My first problem with this is superficial: the error really should be reported on the first import that violates, not the last one, or better yet it should report it on the code that appears between the docstring and the first import since that's really what you're complaining about.

But my bigger problem with E402 is that it should not complain about dunder assigments between the docstring and import. It is completely natural and actually preferred to put things like __version__ and __author__ in this location because they are more visible than if they appeared after the imports. PEP 8 shouldn't be a straightjacket here.

sigmavirus24 commented 9 years ago

@warsaw I agree. I think we need to exclude __var__ from this check for exactly this reason.

lewisc commented 9 years ago

I opened a mail on pthon dev about this

https://mail.python.org/pipermail/python-dev/2015-March/138843.html

Is Guido's response, there is some discussion after that.

for reference

warsaw commented 9 years ago

On Aug 19, 2015, at 09:26 AM, Lewis Coates wrote:

I opened a mail on pthon dev about this

https://mail.python.org/pipermail/python-dev/2015-March/138843.html

Is Guido's response, there is some discussion after that.

Here's Guido's relevant quote IMO:

That said, if an official answer is required, common sense would suggest
that __version__ should go before the imports. (I would put it before the
docstring too, except then the docstring wouldn't be a docstring any more.
Go figure.)

Although I tend to put __all__ after imports, I think it's fine to put __version__ and __author__ before imports, if you put them in at all, which I don't but some people do. The pep8 tool should be lenient about dunders between docstrings and imports, IMO.

IanLee1521 commented 8 years ago

@warsaw -- Would this be something that the PEP-8 document could be updated to say explicitly?

warsaw commented 8 years ago

I didn't realize/remember that PEP 8 requires __all__ after imports. See http://bugs.python.org/issue27187

5j9 commented 7 years ago

At the moment PEP8 recommendation is as follows:

Module level dunder names

Module level "dunders" (i.e. names with two leading and two trailing underscores) such as all , author , version , etc. should be placed after the module docstring but before any import statements except from future imports. Python mandates that future-imports must appear in the module before any other code except docstrings.

For example:

"""This is the example module.

This module does stuff. """

from future import barry_as_FLUFL

all = ['a', 'b', 'c'] version = '0.1' author = 'Cardinal Biggles'

import os import sys

https://www.python.org/dev/peps/pep-0008/#module-level-dunder-names

lewisc commented 7 years ago

Thanks!