Problem in 0.14.3 with `noqa` acceptance

[justinmayer]

Prior to https://github.com/PyCQA/flake8-import-order/commit/95fe1b1f2ed1db6051acd8d1163dfb12a6b4784a and the 0.14.3 release, the following lines from pelican/__init__.py produced the expected result (i.e., passes flake8 tests):

# pelican.log has to be the first pelican module to be loaded
# because logging.setLoggerClass has to be called before logging.getLogger
from pelican.log import init  # noqa
from pelican import signals
[... other imports ...]

Post-0.14.3 release, Travis is now reporting a test failure in a recent pull request:

pelican/__init__.py:18:1: I100 Import statements are in the wrong order. from pelican should be before from pelican.log

I'm guessing this change in behavior is an unintended side effect that should be addressed in a subsequent flake8-import-order release?

[pgjones]

I can reproduce the change in behaviour. It is explained by the fact that pre 0.14.3 releases would completely ignore lines with # noqa, whereas 0.14.3 recognises all the lines yet will silence errors for lines with a # noqa. The error in both versions only exists due to the addition of the signals line as both versions check I100 for a each line against the preceding line.

I think the correct behaviour is to accept a # noqa on either of the two lines in violation of a I100, as you can argue that they are both out of order and ignoring either is ok.

I'll add this together with the ability to specify which errors should be ignored i.e. # noqa: I???.

[justinmayer]

Many thanks, Phil. Much appreciated. (^_^)

[pgjones]

Having looked into implementing this, I am now conflicted as I think this was a bug that is now fixed. What makes me conflicted is that the inline noqa should silence an error reported on that line, so it seems like it would be confusing to allow inline noqas to apply to surrounding lines.

Is it much effort to change pelican to work with the new behaviour?

[pgjones]

The specification error ignoring (# noqa: I???) is now in master.

[avaris]

Changing this won't be an issue but I think this new behavior will look more confusing. The case is something like:

from X.B import b  # <- this was supposed to be at the end, but moved to top
from X import x    # <- now this innocent line has to get a noqa
from X.A import a

It's not just the top two lines are out of order. If you switch these two, you'll still get an error for the third line. First line should go to the bottom to be in perfect order.

[pgjones]

You are correct, but this is a limitation of this checker at the moment. So I've added something to the limitations section. It would be nice to improve this, likely in consideration of #99.

[justinmayer]

For now I've addressed the problem by adjusting things in Pelican via https://github.com/getpelican/pelican/commit/8ebc120f365a4908c060fbc3e66d656e691be702. Many thanks for the great communication and for being so responsive. 😁