Lines with a NOQA comment is grouped separately with 5.0.4

[ljodal]

Hi, I'm just testing version 5.0.4 with the black profile on one of the code bases where we use Isort. I see that it groups lines with a NOQA comment separately from other imports, but not lines with a lower case noqa comment. These changes also appear to only apply to top-level imports. Is this intentional?

~/Code/isort-test → cat a.py
def foo(): pass
def bar(): pass
def baz(): pass

~/Code/isort-test → cat b.py
from a import foo
from a import bar  # NOQA
from a import baz  # noqa

~/Code/isort-test → isort --profile black b.py
Fixing /Users/sigurdljodal/Code/isort-test/b.py

~/Code/isort-test → cat b.py
from a import baz  # noqa
from a import foo

from a import bar  # NOQA

I expected the noqa-lines to be sorted together with the rest of the imports from the same module. Running on Python 3.7.5. Let me know if you need anything more from me :)

[Sahil-Ajmera]

+1 to this . Facing the same issue

[tombo315]

My observation is the NOQA directive under isort is case-sensitive, and inappropriately clashes with flake8.

In @ljodal's example, # NOQA: F401 would have the same effect and matches the code line here:
https://github.com/timothycrosley/isort/blob/develop/isort/parse.py#L56

I'd be happy to PR this myself, but changing pragma's seems like something left to someone more knowledgeable.
Perhaps something along the lines of:

line = line.lower()
'noqa   '.rstrip().endswith('noqa')

[timothycrosley]

My initial inclination is to just remove the NOQA functionality from isort, curious what others thoughts are on this? isort has its own comments to handle skipping, etc - and using or abiding by overloaded ones seems likely to lead to ongoing confusion. Especially since isort is a formatter firstly, and a quality assurance tool second.

[ljodal]

I must admit that I don't have a clear view of what the NOQA support in isort is supposed to do, but we're mostly using it to silence flake8 warnings for unused imports (for stuff that have side effects or for stringly typed stuff). I'd still like those imports to be sorted along with the rest (just like they were pre isort 5) :)

[timothycrosley]

Found the background information: a4a2239

#1065
#679

I can see the logic of both sides, but I think the default should be to skip the QA imports like isort did in 4.x.x and only enable them again if a setting explicitly asks for the flag to be honored

[timothycrosley]

The latest release restores the old behaviour while allowing noqa comments to optionally be utilized for those that want that behaviour via an --honor-noqa setting.

[ljodal]

Looks good in our codebase now, thanks for the quick fix!