PyCQA / flake8-import-order

Flake8 plugin that checks import order against various Python Style Guides
GNU Lesser General Public License v3.0
278 stars 72 forks source link

Regression in 0.15 with I202 (newline in imports) #122

Closed dhermes closed 6 years ago

dhermes commented 6 years ago

For a given file repro.py, there is a regression:

$ virtualenv venv
$ venv/bin/pip install flake8 flake8-import-order numpy
$ venv/bin/python repro.py
Here are my imports:
- matplotlib.pyplot: None
- numpy: <module 'numpy' from '.../venv/lib/python3.6/site-packages/numpy/__init__.py'>
- seaborn: None
- six: <module 'six' from '.../venv/lib/python3.6/site-packages/six.py'>
$
$ venv/bin/pip show flake8-import-order
Name: flake8-import-order
Version: 0.15
$ venv/bin/flake8 repro.py --import-order-style=google
repro.py:10:1: I202 Additional newline in a section of imports.
$
$ venv/bin/pip install 'flake8-import-order==0.14.3'
$ venv/bin/flake8 repro.py --import-order-style=google
$

# repro.py
try:
    import matplotlib.pyplot as plt
except ImportError:
    plt = None
import numpy as np
try:
    import seaborn  # pylint: disable=unused-import
except ImportError:
    seaborn = None
import six

def main():
    """This is the entry point."""
    print('Here are my imports:')
    print('- matplotlib.pyplot: {}'.format(plt))
    print('- numpy: {}'.format(np))
    print('- seaborn: {}'.format(seaborn))
    print('- six: {}'.format(six))

if __name__ == '__main__':
    main()

I don't have any preference for how I do my imports, so I'm happy to change the imports to "conform" to what is expected. Are there equivalent docs? My logic was "keep them in alphabetical order and catch the potential ImportError-s as they come".

I have "fixed" this issue for now in a real code base just by ignoring the error.

pgjones commented 6 years ago

I think this may be due to

Correct the flake8 entrypoint to report all I errors, this may result in I2XX errors being reported that were absent previously.

rather than a regression.

I this case it is a limitation of this checker that it doesn't pick up the try-except lines (nor the imports within) and hence just sees:

...
import numpy as np

import six

and therefore sees an extra line(s). I would write your imports as:

# repro.py
try:
    import matplotlib.pyplot as plt
except ImportError:
    plt = None
try:
    import seaborn  # pylint: disable=unused-import
except ImportError:
    seaborn = None
import numpy as np
import six

to work around this. Although I think a # noqa: I202 is equally sensible.

dhermes commented 6 years ago

@pgjones Thanks for the quick reply!

  1. What about my OCD need for alphabetical imports?
  2. Is it correct to conclude that try/except are not considered to be imports in any way? Is that bug/"feature request"-worthy?
pgjones commented 6 years ago

I think the noqa is maybe the best I can offer for 1. :), as for 2. I think it could be a feature request but I am unsure how best to implement it - feel free to suggest a PR. I'll close this, as I think we agree this isn't a bug (although reopen if you disagree).

touilleMan commented 6 years ago

Hi, Got the same trouble with a group of imports separated by a block of comments (also see the travis job)

pgjones commented 6 years ago

I think the current master branch fixes the block of comments issue, I'll release it soon.