Repository: timothycrosley/isort
Pull request #843 (2019-02-25 23:23:47)

Added NOQA to lines to be skipped 
Expected behavior is now that all isort:skip and NOQA imports will be moved to bottom, unsorted.


Patch:
diff --git a/isort/isort.py b/isort/isort.py
index c745639f..b717eab9 100644
--- a/isort/isort.py
+++ b/isort/isort.py
@@ -37,7 +37,6 @@ from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Mapping, Optional,
 from . import settings
 from .finders import FindersManager
 from .natural import nsorted
-from .settings import WrapModes
 
 if TYPE_CHECKING:
     from mypy_extensions import TypedDict
@@ -264,7 +263,7 @@ class SortImports(object):
     @staticmethod
     def _import_type(line: str) -> Optional[str]:
         """If the current line is an import line it will return its type (from or straight)"""
-        if "isort:skip" in line:
+        if "isort:skip" in line or "NOQA" in line:
             return None
         elif line.startswith('import '):
             return "straight"
@@ -335,7 +334,7 @@ class SortImports(object):
             Returns an import wrapped to the specified line-length, if possible.
         """
         wrap_mode = self.config['multi_line_output']
-        if len(line) > self.config['line_length'] and wrap_mode != WrapModes.NOQA:
+        if len(line) > self.config['line_length'] and wrap_mode != settings.WrapModes.NOQA:
             line_without_comment = line
             comment = None
             if '#' in line:
@@ -358,9 +357,10 @@ class SortImports(object):
                         output = "{0}{1}({2}{3}{4}{5})".format(
                             line, splitter, self.line_separator, cont_line,
                             "," if self.config['include_trailing_comma'] else "",
-                            self.line_separator if wrap_mode in {WrapModes.VERTICAL_HANGING_INDENT,
-                                                                 WrapModes.VERTICAL_GRID_GROUPED}
-                            else "")
+                            self.line_separator if wrap_mode in (
+                                settings.WrapModes.VERTICAL_HANGING_INDENT,
+                                settings.WrapModes.VERTICAL_GRID_GROUPED,
+                            ) else "")
                         lines = output.split(self.line_separator)
                         if self.config['comment_prefix'] in lines[-1] and lines[-1].endswith(')'):
                             line, comment = lines[-1].split(self.config['comment_prefix'], 1)
@@ -491,20 +491,20 @@ class SortImports(object):
 
                     # If line too long AND have imports AND we are NOT using GRID or VERTICAL wrap modes
                     if (len(import_statement) > self.config['line_length'] and len(from_import_section) > 0 and
-                            self.config['multi_line_output'] not in (settings.WrapModes.GRID, settings.WrapModes.VERTICAL)):
+                            self.config['multi_line_output'] not in (1, 0)):
                         do_multiline_reformat = True
 
                     if do_multiline_reformat:
                         import_statement = self._multi_line_reformat(import_start, from_import_section, comments)
-                        if self.config['multi_line_output'] == settings.WrapModes.GRID:
-                            self.config['multi_line_output'] = settings.WrapModes.VERTICAL_GRID
+                        if self.config['multi_line_output'] == 0:
+                            self.config['multi_line_output'] = 4
                             try:
                                 other_import_statement = self._multi_line_reformat(import_start, from_import_section, comments)
                                 if (max(len(x)
                                         for x in import_statement.split('\n')) > self.config['line_length']):
                                     import_statement = other_import_statement
                             finally:
-                                self.config['multi_line_output'] = settings.WrapModes.GRID
+                                self.config['multi_line_output'] = 0
                     if not do_multiline_reformat and len(import_statement) > self.config['line_length']:
                         import_statement = self._wrap(import_statement)
 
@@ -515,7 +515,7 @@ class SortImports(object):
                     section_output.append(import_statement)
 
     def _multi_line_reformat(self, import_start: str, from_imports: List[str], comments: Sequence[str]) -> str:
-        output_mode = self.config['multi_line_output'].name.lower()
+        output_mode = settings.WrapModes._fields[self.config['multi_line_output']].lower()
         formatter = getattr(self, "_output_" + output_mode, self._output_grid)
         dynamic_indent = " " * (len(import_start) + 1)
         indent = self.config['indent']
@@ -734,13 +734,17 @@ class SortImports(object):
         line_length: int,
         comments: Sequence[str]
     ) -> str:
+        trailing = ""
+        if len(imports) > 1 and self.config['include_trailing_comma']:
+            trailing = ','
+
         return "{0}({1}{2}{3}{4}{5}{2})".format(
             statement,
             self._add_comments(comments),
             self.line_separator,
             indent,
             ("," + self.line_separator + indent).join(imports),
-            "," if self.config['include_trailing_comma'] else "",
+            trailing,
          )
 
     def _output_vertical_grid_common(
diff --git a/isort/main.py b/isort/main.py
index a2d4b935..b05cb48c 100644
--- a/isort/main.py
+++ b/isort/main.py
@@ -233,7 +233,7 @@ def parse_args(argv: Optional[Sequence[str]] = None) -> Dict[str, Any]:
                         help="Forces line endings to the specified value. If not set, values will be guessed per-file.")
     parser.add_argument('-ls', '--length-sort', help='Sort imports by their string length.',
                         dest='length_sort', action='store_true')
-    parser.add_argument('-m', '--multi-line', dest='multi_line_output', type=WrapModes.from_string,
+    parser.add_argument('-m', '--multi-line', dest='multi_line_output', type=int, choices=range(len(WrapModes)),
                         help='Multi line output (0-grid, 1-vertical, 2-hanging, 3-vert-hanging, 4-vert-grid, '
                         '5-vert-grid-grouped, 6-vert-grid-grouped-no-comma).')
     inline_args_group.add_argument('-nis', '--no-inline-sort', dest='no_inline_sort', action='store_true',
diff --git a/isort/settings.py b/isort/settings.py
index c1d17802..887a2572 100644
--- a/isort/settings.py
+++ b/isort/settings.py
@@ -23,13 +23,13 @@ OTHER DEALINGS IN THE SOFTWARE.
 
 """
 import configparser
-import enum
 import fnmatch
 import os
 import posixpath
 import re
 import stat
 import warnings
+from collections import namedtuple
 from distutils.util import strtobool
 from functools import lru_cache
 from typing import Any, Dict, Iterable, List, Mapping, MutableMapping, Type
@@ -55,21 +55,9 @@ safety_exclude_re = re.compile(
     r"/(\.eggs|\.git|\.hg|\.mypy_cache|\.nox|\.tox|\.venv|_build|buck-out|build|dist|lib/python[0-9].[0-9]+)/"
 )
 
-
-class WrapModes(enum.Enum):
-    GRID = 0  # 0
-    VERTICAL = 1
-    HANGING_INDENT = 2
-    VERTICAL_HANGING_INDENT = 3
-    VERTICAL_GRID = 4
-    VERTICAL_GRID_GROUPED = 5
-    VERTICAL_GRID_GROUPED_NO_COMMA = 6
-    NOQA = 7
-
-    @staticmethod
-    def from_string(value: str) -> 'WrapModes':
-        return WrapModes(int(value))
-
+_wrap_mode_strings = ('GRID', 'VERTICAL', 'HANGING_INDENT', 'VERTICAL_HANGING_INDENT', 'VERTICAL_GRID', 'VERTICAL_GRID_GROUPED',
+                      'VERTICAL_GRID_GROUPED_NO_COMMA', 'NOQA')
+WrapModes = namedtuple('WrapModes', _wrap_mode_strings)(*range(len(_wrap_mode_strings)))  # type: Any
 
 # Note that none of these lists must be complete as they are simply fallbacks for when included auto-detection fails.
 default = {'force_to_top': [],
@@ -128,7 +116,7 @@ default = {'force_to_top': [],
                                       'zipimport', 'zlib'],
            'known_third_party': ['google.appengine.api'],
            'known_first_party': [],
-           'multi_line_output': WrapModes.GRID,
+           'multi_line_output': WrapModes.GRID,  # type: ignore
            'forced_separate': [],
            'indent': ' ' * 4,
            'comment_prefix': '  #',


Test patch:
diff --git a/test_isort.py b/test_isort.py
index 355c6fba..e4566736 100644
--- a/test_isort.py
+++ b/test_isort.py
@@ -1304,7 +1304,7 @@ def test_include_trailing_comma():
     ).output
     assert test_output_wrap_single_import_vertical_indent == (
         "from third_party import (\n"
-        "    lib1,\n"
+        "    lib1\n"
         ")\n"
     )
 
@@ -1978,17 +1978,14 @@ def test_no_additional_lines_issue_358():
                        '    print_function,\n'
                        '    unicode_literals\n'
                        ')\n')
-    test_output = SortImports(file_contents=test_input, multi_line_output=WrapModes.VERTICAL_HANGING_INDENT,
-                              line_length=20).output
+    test_output = SortImports(file_contents=test_input, multi_line_output=3, line_length=20).output
     assert test_output == expected_output
 
-    test_output = SortImports(file_contents=test_output, multi_line_output=WrapModes.VERTICAL_HANGING_INDENT,
-                              line_length=20).output
+    test_output = SortImports(file_contents=test_output, multi_line_output=3, line_length=20).output
     assert test_output == expected_output
 
     for attempt in range(5):
-        test_output = SortImports(file_contents=test_output, multi_line_output=WrapModes.VERTICAL_HANGING_INDENT,
-                                  line_length=20).output
+        test_output = SortImports(file_contents=test_output, multi_line_output=3, line_length=20).output
         assert test_output == expected_output
 
     test_input = ('"""This is a docstring"""\n'
@@ -2009,17 +2006,14 @@ def test_no_additional_lines_issue_358():
                        '    print_function,\n'
                        '    unicode_literals\n'
                        ')\n')
-    test_output = SortImports(file_contents=test_input, multi_line_output=WrapModes.VERTICAL_HANGING_INDENT,
-                              line_length=20).output
+    test_output = SortImports(file_contents=test_input, multi_line_output=3, line_length=20).output
     assert test_output == expected_output
 
-    test_output = SortImports(file_contents=test_output, multi_line_output=WrapModes.VERTICAL_HANGING_INDENT,
-                              line_length=20).output
+    test_output = SortImports(file_contents=test_output, multi_line_output=3, line_length=20).output
     assert test_output == expected_output
 
     for attempt in range(5):
-        test_output = SortImports(file_contents=test_output, multi_line_output=WrapModes.VERTICAL_HANGING_INDENT,
-                                  line_length=20).output
+        test_output = SortImports(file_contents=test_output, multi_line_output=3, line_length=20).output
         assert test_output == expected_output
 
 
@@ -2188,7 +2182,7 @@ def test_alias_using_paren_issue_466():
     expected_output = ('from django.db.backends.mysql.base import (\n'
                        '    DatabaseWrapper as MySQLDatabaseWrapper\n'
                        ')\n')
-    assert SortImports(file_contents=test_input, line_length=50, multi_line_output=WrapModes.VERTICAL_GRID_GROUPED,
+    assert SortImports(file_contents=test_input, line_length=50, multi_line_output=5,
                        use_parentheses=True).output == expected_output
 
 
@@ -2310,8 +2304,7 @@ def test_long_import_wrap_support_with_mode_2():
     """Test to ensure mode 2 still allows wrapped imports with slash"""
     test_input = ('from foobar.foobar.foobar.foobar import \\\n'
                   '    an_even_longer_function_name_over_80_characters\n')
-    assert SortImports(file_contents=test_input, multi_line_output=WrapModes.HANGING_INDENT,
-                       line_length=80).output == test_input
+    assert SortImports(file_contents=test_input, multi_line_output=2, line_length=80).output == test_input
 
 
 def test_pylint_comments_incorrectly_wrapped_issue_571():
@@ -2753,3 +2746,22 @@ def test_unwrap_issue_762():
     test_input = ('from os.\\\n'
                   '    path import (join, split)')
     assert SortImports(file_contents=test_input).output == 'from os.path import join, split\n'
+
+
+def test_noqa_issue_679():
+    # Test to ensure that NOQA notation is being observed as expected
+    test_input = ('import os\n'
+                  '\n'
+                  'import requestsss\n'
+                  'import zed # NOQA\n'
+                  'import ujson # NOQA\n'
+                  '\n'
+                  'import foo')
+    test_output = ('import os\n'
+                   '\n'
+                   'import foo\n'
+                   'import requestsss\n'
+                   '\n'
+                   'import zed # NOQA\n'
+                   'import ujson # NOQA\n')
+    assert SortImports(file_contents=test_input).output == test_output