From f86fcf944324ebf396a0730b877b2e3f18be8fa5 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sun, 27 Aug 2023 23:36:03 +0530 Subject: [PATCH 01/10] add sort imports function --- bin/tidy-imports | 11 ++++++----- lib/python/pyflyby/_imports2s.py | 4 ++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/bin/tidy-imports b/bin/tidy-imports index 7639e4b5..6bed81ae 100755 --- a/bin/tidy-imports +++ b/bin/tidy-imports @@ -27,10 +27,10 @@ import sys import os from pyflyby._cmdline import hfmt, parse_args, process_actions -from pyflyby._imports2s import (canonicalize_imports, - fix_unused_and_missing_imports, - replace_star_imports, - transform_imports) +from pyflyby._imports2s import (canonicalize_imports, + fix_unused_and_missing_imports, + replace_star_imports, + transform_imports, sort_imports) from pyflyby._log import logger import toml @@ -153,12 +153,13 @@ def main(): params=options.params) if options.replace_star_imports: x = replace_star_imports(x, params=options.params) - return fix_unused_and_missing_imports( + x = fix_unused_and_missing_imports( x, params=options.params, add_missing=options.add_missing, remove_unused=options.remove_unused, add_mandatory=options.add_mandatory, ) + return sort_imports(x) process_actions(args, options.actions, modify) diff --git a/lib/python/pyflyby/_imports2s.py b/lib/python/pyflyby/_imports2s.py index 7a554f08..3deaccd8 100644 --- a/lib/python/pyflyby/_imports2s.py +++ b/lib/python/pyflyby/_imports2s.py @@ -534,6 +534,10 @@ def replace_star_imports(codeblock, params=None): return transformer.output(params=params) +def sort_imports(codeblock): + return codeblock + + def transform_imports(codeblock, transformations, params=None): """ Transform imports as specified by ``transformations``. From 1674cc1ebb38c256c0e58c49cdc3ae72f0f0b267 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Mon, 28 Aug 2023 00:03:28 +0530 Subject: [PATCH 02/10] add basic test --- tests/test_cmdline.py | 61 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index e7c549b2..0fe39e49 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -681,3 +681,64 @@ def test_tidy_imports_symlinks_bad_argument(): assert b"error: --symlinks must be one of" in proc_output assert output == input assert symlink_output == input + + +def test_tidy_imports_sorting(): + with tempfile.NamedTemporaryFile(suffix=".py", mode='w+') as f: + f.write(dedent(""" + import numpy + + from pkg1.mod1 import foo + from pkg1.mod2 import bar + from pkg2 import baz + import yy + + from pkg1.mod1 import foo2 + from pkg1.mod3 import quux + from pkg2 import baar + import sympy + import zz + + + zz.foo() + bar() + quux() + foo2() + yy.f() + bar() + foo() + numpy.arange() + baz + baar + sympy + """).lstrip()) + f.flush() + result = pipe([BIN_DIR+"/tidy-imports", f.name]) + expected = dedent(""" + import numpy + + from pkg1.mod1 import foo + from pkg1.mod2 import bar + from pkg2 import baz + import yy + + from pkg1.mod1 import foo2 + from pkg1.mod3 import quux + from pkg2 import baar + import sympy + import zz + + + zz.foo() + bar() + quux() + foo2() + yy.f() + bar() + foo() + numpy.arange() + baz + baar + sympy + """).strip().format(f=f) + assert result == expected From f6fad7fff5919b7a255d7b7c3f5598b4cc52ccf5 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Mon, 28 Aug 2023 11:08:13 +0530 Subject: [PATCH 03/10] add isort --- lib/python/pyflyby/_imports2s.py | 11 +++++++---- setup.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/python/pyflyby/_imports2s.py b/lib/python/pyflyby/_imports2s.py index 3deaccd8..4d20cb1d 100644 --- a/lib/python/pyflyby/_imports2s.py +++ b/lib/python/pyflyby/_imports2s.py @@ -1,9 +1,7 @@ # pyflyby/_imports2s.py. # Copyright (C) 2011-2018 Karl Chen. # License: MIT http://opensource.org/licenses/MIT - - - +import isort from pyflyby._autoimp import scan_for_import_issues from pyflyby._file import FileText, Filename from pyflyby._flags import CompilerFlags @@ -535,7 +533,12 @@ def replace_star_imports(codeblock, params=None): def sort_imports(codeblock): - return codeblock + """ + Sort imports for better grouping. + :param codeblock: + :return: codeblock + """ + return isort.code(codeblock) def transform_imports(codeblock, transformations, params=None): diff --git a/setup.py b/setup.py index 01db9c90..14218edd 100755 --- a/setup.py +++ b/setup.py @@ -221,7 +221,7 @@ def make_distribution(self): "License :: OSI Approved :: MIT License", "Programming Language :: Python", ], - install_requires=["six", "toml", "pathlib ; python_version<'3'"], + install_requires=["six", "toml", "isort", "pathlib ; python_version<'3'"], python_requires=">3.0, !=3.0.*, !=3.1.*, !=3.2.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*, !=3.6.*, <4", tests_require=['pexpect>=3.3', 'pytest', 'epydoc', 'rlipython', 'requests'], cmdclass = { From d2a342eff6935e8ddc3394412525a4ab73d55310 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 12 Sep 2023 20:23:20 +0530 Subject: [PATCH 04/10] insert lines between packages --- lib/python/pyflyby/_imports2s.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/python/pyflyby/_imports2s.py b/lib/python/pyflyby/_imports2s.py index 4d20cb1d..c8961d39 100644 --- a/lib/python/pyflyby/_imports2s.py +++ b/lib/python/pyflyby/_imports2s.py @@ -1,6 +1,8 @@ # pyflyby/_imports2s.py. # Copyright (C) 2011-2018 Karl Chen. # License: MIT http://opensource.org/licenses/MIT +from collections import defaultdict + import isort from pyflyby._autoimp import scan_for_import_issues from pyflyby._file import FileText, Filename @@ -538,7 +540,33 @@ def sort_imports(codeblock): :param codeblock: :return: codeblock """ - return isort.code(codeblock) + sorted_imports = isort.code(str(codeblock), force_sort_within_sections=True) + # Step 1: Split the input string into a list of lines + lines = sorted_imports.strip().split('\n') + + # Step 2: Identify groups of imports and keep track of their line numbers + pkg_lines = defaultdict(list) + line_pkg_dict = {} + for i, line in enumerate(lines): + match = re.match(r'(from (\w+)|import (\w+))', line) + current_pkg = match.groups()[1:3] + current_pkg = current_pkg[0] if current_pkg[0] is not None else current_pkg[1] + + pkg_lines[current_pkg].append(i) + line_pkg_dict[i] = current_pkg + + # Step 3: Create the output list of lines with blank lines around groups with more than one import + output_lines = [] + for i, line in enumerate(lines): + if i > 0 and line_pkg_dict[i] != line_pkg_dict[i-1] and len(pkg_lines[line_pkg_dict[i]]) > 1: + output_lines.append('') + output_lines.append(line) + if i < len(lines) - 1 and line_pkg_dict[i] != line_pkg_dict[i+1] and len(pkg_lines[line_pkg_dict[i]]) > 1: + output_lines.append('') + + # Step 4: Join the lines to create the output string + sorted_output_str = '\n'.join(output_lines) + return PythonBlock(sorted_output_str) def transform_imports(codeblock, transformations, params=None): From 2b1f9cdbfeace556553294fa5ad73b0921c598ef Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Tue, 12 Sep 2023 20:32:14 +0530 Subject: [PATCH 05/10] fix is not matching pattern --- bin/tidy-imports | 7 ++++--- lib/python/pyflyby/_imports2s.py | 14 +++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/bin/tidy-imports b/bin/tidy-imports index 6bed81ae..09ce2bef 100755 --- a/bin/tidy-imports +++ b/bin/tidy-imports @@ -146,8 +146,6 @@ def main(): options, args = parse_args( _add_opts_and_defaults, import_format_params=True, modify_action_params=True, defaults=default_config) def modify(x): - if options.canonicalize: - x = canonicalize_imports(x, params=options.params) if options.transformations: x = transform_imports(x, options.transformations, params=options.params) @@ -159,7 +157,10 @@ def main(): remove_unused=options.remove_unused, add_mandatory=options.add_mandatory, ) - return sort_imports(x) + sx = sort_imports(x) + if options.canonicalize: + sx = canonicalize_imports(x, params=options.params) + return sx process_actions(args, options.actions, modify) diff --git a/lib/python/pyflyby/_imports2s.py b/lib/python/pyflyby/_imports2s.py index c8961d39..8d42f08d 100644 --- a/lib/python/pyflyby/_imports2s.py +++ b/lib/python/pyflyby/_imports2s.py @@ -549,19 +549,19 @@ def sort_imports(codeblock): line_pkg_dict = {} for i, line in enumerate(lines): match = re.match(r'(from (\w+)|import (\w+))', line) - current_pkg = match.groups()[1:3] - current_pkg = current_pkg[0] if current_pkg[0] is not None else current_pkg[1] - - pkg_lines[current_pkg].append(i) - line_pkg_dict[i] = current_pkg + if match: + current_pkg = match.groups()[1:3] + current_pkg = current_pkg[0] if current_pkg[0] is not None else current_pkg[1] + pkg_lines[current_pkg].append(i) + line_pkg_dict[i] = current_pkg # Step 3: Create the output list of lines with blank lines around groups with more than one import output_lines = [] for i, line in enumerate(lines): - if i > 0 and line_pkg_dict[i] != line_pkg_dict[i-1] and len(pkg_lines[line_pkg_dict[i]]) > 1: + if i > 0 and line_pkg_dict.get(i) != line_pkg_dict.get(i-1) and len(pkg_lines[line_pkg_dict.get(i)]) > 1: output_lines.append('') output_lines.append(line) - if i < len(lines) - 1 and line_pkg_dict[i] != line_pkg_dict[i+1] and len(pkg_lines[line_pkg_dict[i]]) > 1: + if i < len(lines) - 1 and line_pkg_dict.get(i) != line_pkg_dict.get(i+1) and len(pkg_lines[line_pkg_dict.get(i)]) > 1: output_lines.append('') # Step 4: Join the lines to create the output string From a9101ad42d879126d1da6f790723014472b3d8e6 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Wed, 13 Sep 2023 14:17:53 +0530 Subject: [PATCH 06/10] fix sorted imports bug --- bin/tidy-imports | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bin/tidy-imports b/bin/tidy-imports index 09ce2bef..7737a80d 100755 --- a/bin/tidy-imports +++ b/bin/tidy-imports @@ -145,6 +145,7 @@ def main(): parser.set_defaults(**default_config) options, args = parse_args( _add_opts_and_defaults, import_format_params=True, modify_action_params=True, defaults=default_config) + def modify(x): if options.transformations: x = transform_imports(x, options.transformations, @@ -157,10 +158,10 @@ def main(): remove_unused=options.remove_unused, add_mandatory=options.add_mandatory, ) - sx = sort_imports(x) + sorted_imports = sort_imports(x) if options.canonicalize: - sx = canonicalize_imports(x, params=options.params) - return sx + sorted_imports = canonicalize_imports(sorted_imports, params=options.params) + return sorted_imports process_actions(args, options.actions, modify) From ae61670226ab84f0239c1da0c9342d0aaa1de1f2 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Wed, 13 Sep 2023 14:28:32 +0530 Subject: [PATCH 07/10] fix sorting test --- tests/test_cmdline.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 0fe39e49..47d67090 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -717,18 +717,15 @@ def test_tidy_imports_sorting(): expected = dedent(""" import numpy - from pkg1.mod1 import foo + from pkg1.mod1 import foo, foo2 from pkg1.mod2 import bar - from pkg2 import baz - import yy - - from pkg1.mod1 import foo2 from pkg1.mod3 import quux - from pkg2 import baar + + from pkg2 import baar, baz import sympy + import yy import zz - zz.foo() bar() quux() From e033ed56780a47f633fd2fe75300b16cec115977 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Wed, 13 Sep 2023 14:35:12 +0530 Subject: [PATCH 08/10] formatting isort call --- lib/python/pyflyby/_imports2s.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/python/pyflyby/_imports2s.py b/lib/python/pyflyby/_imports2s.py index 8d42f08d..5d1e40cb 100644 --- a/lib/python/pyflyby/_imports2s.py +++ b/lib/python/pyflyby/_imports2s.py @@ -540,7 +540,10 @@ def sort_imports(codeblock): :param codeblock: :return: codeblock """ - sorted_imports = isort.code(str(codeblock), force_sort_within_sections=True) + sorted_imports = isort.code( + str(codeblock), + force_sort_within_sections=True, + ) # Step 1: Split the input string into a list of lines lines = sorted_imports.strip().split('\n') From 4ff236b55f5fbd3aa10bd9cd693d676840a7a334 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Wed, 13 Sep 2023 14:46:45 +0530 Subject: [PATCH 09/10] document isort call --- lib/python/pyflyby/_imports2s.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/python/pyflyby/_imports2s.py b/lib/python/pyflyby/_imports2s.py index 5d1e40cb..cdc72a7d 100644 --- a/lib/python/pyflyby/_imports2s.py +++ b/lib/python/pyflyby/_imports2s.py @@ -542,7 +542,10 @@ def sort_imports(codeblock): """ sorted_imports = isort.code( str(codeblock), + # To sort all the import in lexicographic order force_sort_within_sections=True, + # This is done below + lines_between_sections=0 ) # Step 1: Split the input string into a list of lines lines = sorted_imports.strip().split('\n') From 4717785e64c5f0b63a90d6da563ba332fb74fa70 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Mon, 25 Sep 2023 10:54:34 +0100 Subject: [PATCH 10/10] fix failing tests --- lib/python/pyflyby/_imports2s.py | 5 +++-- tests/test_cmdline.py | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/python/pyflyby/_imports2s.py b/lib/python/pyflyby/_imports2s.py index cdc72a7d..5a33879f 100644 --- a/lib/python/pyflyby/_imports2s.py +++ b/lib/python/pyflyby/_imports2s.py @@ -545,10 +545,11 @@ def sort_imports(codeblock): # To sort all the import in lexicographic order force_sort_within_sections=True, # This is done below - lines_between_sections=0 + lines_between_sections=0, + lines_after_imports=1 ) # Step 1: Split the input string into a list of lines - lines = sorted_imports.strip().split('\n') + lines = sorted_imports.split('\n') # Step 2: Identify groups of imports and keep track of their line numbers pkg_lines = defaultdict(list) diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 47d67090..23c0af88 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -121,6 +121,7 @@ def foo(): foo() + os + sys import a import c + a, c ''').lstrip() assert result == expected_result @@ -138,6 +139,7 @@ def test_tidy_imports_no_add_no_remove_1(): import a import b import c + a, c, os, sys ''').strip() assert result == expected @@ -409,6 +411,7 @@ def test_tidy_imports_query_no_change_1(): input = dedent(''' from __future__ import absolute_import, division import x1 + x1 ''') with tempfile.NamedTemporaryFile(suffix=".py", mode='w+') as f: @@ -446,6 +449,7 @@ def test_tidy_imports_query_y_1(): expected = dedent(""" from __future__ import absolute_import, division import x1 + x1 """) assert output == expected