Allow to configure `isort` through pyproject.toml #13

Mystic-Mirage · 2020-07-02T23:22:58Z

Now it's possible to customize isort parameters using the same pyproject.toml that contains black configuration.

akaihola · 2020-07-03T12:40:36Z

This is another great addition!

The patch touches a number of the same places as #11.

Assuming both are wanted, which one do you think should be merged first, leaving conflicts to be solved in the other one?

I'm ok with either. It makes sense to start reviewing the one to be merged first, and postponing review of the other after solving the conflicts.

Mystic-Mirage · 2020-07-03T13:04:56Z

I'm ok with historical order -- let's merge #11 and then I'll solve conflicts here.

akaihola · 2020-07-03T15:34:49Z

Ok, I merged #11 to #4, will merge that too next. I'll wait for your edits before reviewing.

akaihola · 2020-07-03T15:52:26Z

Merged #4 as well.

Mystic-Mirage · 2020-07-03T17:42:15Z

Solved conflicts

CHANGES.rst

pyproject.toml

akaihola · 2020-07-03T19:43:31Z

src/darker/black_diff.py

@@ -35,7 +35,7 @@
 import logging
 from functools import lru_cache
 from pathlib import Path
-from typing import Dict, List, Optional, Tuple, Union
+from typing import Dict, List, Optional, Tuple, Union, cast


(I messed up the order of these comments, see the one beginning with "Now that we have three different types" further down in black_diff.py first to understand what this all is about... sorry...)

And after logger = logging.getLogger(__name__) on line 43 we could add

class BlackArgs(TypedDict, total=False): config: str line_length: int skip_string_normalization: bool

On line 47, we would use the BlackArgs type:

def read_black_config(src: Path, value: Optional[str]) -> BlackArgs:

And we'd need to adjust src/darker/tests/test_black_diff.py by importing BlackArgs and using it to create the empty dict when calling run_black:

from darker.black_diff import BlackArgs, read_black_config, run_black # ... def test_run_black(tmpdir): src_contents = "print ( '42' )\n" result = run_black(Path(tmpdir / "src.py"), src_contents, BlackArgs()) assert result == ['print("42")']

With BlackArgs, the signature of read_black_config() on line 47 should change, and the return value on line 58 needs to be cast:

def read_black_config(src: Path, value: Optional[str]) -> BlackArgs: # ... return cast( BlackArgs, { key: value for key, value in (context.default_map or {}).items() if key in ["line_length", "skip_string_normalization"] }, )

akaihola · 2020-07-03T19:45:08Z

src/darker/black_diff.py

@@ -63,7 +63,7 @@ def read_black_config(src: Path, value: Optional[str]) -> Dict[str, Union[bool,


 def run_black(
-    src: Path, src_contents: str, black_args: Dict[str, Union[bool, int]]
+    src: Path, src_contents: str, black_args: Dict[str, Union[bool, int, str]]


We could use the new type here, too:

Suggested change

src: Path, src_contents: str, black_args: Dict[str, Union[bool, int, str]]

src: Path, src_contents: str, black_args: BlackArgs

akaihola · 2020-07-03T19:45:52Z

src/darker/black_diff.py

+    args = cast(Dict[str, Union[bool, int]], black_args)
+    combined_args = {**defaults, **args}


There would be no need to cast here:

Suggested change

args = cast(Dict[str, Union[bool, int]], black_args)

combined_args = {**defaults, **args}

combined_args = {**defaults, **black_args}

akaihola · 2020-07-03T20:32:51Z

src/darker/black_diff.py

@@ -35,7 +35,7 @@
 import logging
 from functools import lru_cache
 from pathlib import Path
-from typing import Dict, List, Optional, Tuple, Union
+from typing import Dict, List, Optional, Tuple, Union, cast


Now that we have three different types in black_args, maybe it's time to use TypedDict. This way we can get rid of the need to cast.

Suggested change

from typing import Dict, List, Optional, Tuple, Union, cast

import sys

from typing import List, Optional, cast

if sys.version_info >= (3, 8):

from typing import TypedDict

else:

from typing_extensions import TypedDict

akaihola · 2020-07-03T20:36:24Z

src/darker/import_sorting.py

+from typing import Dict, List, Optional, Tuple, Union, cast
+
+from black import find_project_root


We could use TypedDict for IsortSettings as well.

Suggested change

from typing import Dict, List, Optional, Tuple, Union, cast

from black import find_project_root

import sys

from typing import Optional, cast

from black import find_project_root

if sys.version_info >= (3, 8):

from typing import TypedDict

else:

from typing_extensions import TypedDict

To support TypedDict in Python <3.8, we need this in setup.cfg:

install_requires = black typing-extensions ; python_version < "3.8"

akaihola · 2020-07-03T20:37:24Z

src/darker/import_sorting.py


 logger = logging.getLogger(__name__)

+IsortSettings = Dict[str, Union[bool, int, List[str], str, Tuple[str], None]]


This would be the type definition for IsortSettings. No need to define types for keys we don't use.

Suggested change

IsortSettings = Dict[str, Union[bool, int, List[str], str, Tuple[str], None]]

class IsortSettings(TypedDict):

line_length: int

akaihola · 2020-07-03T20:44:06Z

src/darker/import_sorting.py

+        _update_with_config_file,
+        default,
+        from_path,
+    )


It becomes really weird to have these functions used in this module without the isort.settings. prefix. But fixing that would require rethinking the way we run without isort installed and mock isort in unit tests. So let's think about that separately if at all.

changed to import isort.settings as isort_settings

akaihola · 2020-07-03T21:01:51Z

src/darker/import_sorting.py

-        line_length=88,
-        quiet=True,
+        "SortImports(file_contents=..., check=True, {})".format(
+            ", ".join(f"{k}={v}" for k, v in isort_settings.items())


This becomes seriously verbose in debug mode:

DEBUG:darker.import_sorting:SortImports(file_contents=..., check=True, force_to_top=[], skip=[], skip_glob=[], line_length=88, wrap_length=0, line_ending=None, sections=('FUTURE', 'STDLIB', 'THIRDPARTY', 'FIRSTPARTY', 'LOCALFOLDER'), no_sections=False, known_future_library=['__future__'], known_standard_library=['AL', 'BaseHTTPServer', 'Bastion', 'CGIHTTPServer', 'Carbon', 'ColorPicker', 'ConfigParser', 'Cookie', 'DEVICE', 'DocXMLRPCServer', 'EasyDialogs', 'FL', 'FrameWork', 'GL', 'HTMLParser', 'MacOS', 'MimeWriter', 'MiniAEFrame', 'Nav', 'PixMapWrapper', 'Queue', 'SUNAUDIODEV', 'ScrolledText', 'SimpleHTTPServer', 'SimpleXMLRPCServer', 'SocketServer', 'StringIO', 'Tix', 'Tkinter', 'UserDict', 'UserList', 'UserString', 'W', '__builtin__', 'abc', 'aepack', 'aetools', 'aetypes', 'aifc', 'al', 'anydbm', 'applesingle', 'argparse', 'array', 'ast', 'asynchat', 'asyncio', 'asyncore', 'atexit', 'audioop', 'autoGIL', 'base64', 'bdb', 'binascii', 'binhex', 'bisect', 'bsddb', 'buildtools', 'builtins', 'bz2', 'cPickle', 'cProfile', 'cStringIO', 'calendar', 'cd', 'cfmfile', 'cgi', 'cgitb', 'chunk', 'cmath', 'cmd', 'code', 'codecs', 'codeop', 'collections', 'colorsys', 'commands', 'compileall', 'compiler', 'concurrent', 'configparser', 'contextlib', 'contextvars', 'cookielib', 'copy', 'copy_reg', 'copyreg', 'crypt', 'csv', 'ctypes', 'curses', 'dataclasses', 'datetime', 'dbhash', 'dbm', 'decimal', 'difflib', 'dircache', 'dis', 'distutils', 'dl', 'doctest', 'dumbdbm', 'dummy_thread', 'dummy_threading', 'email', 'encodings', 'ensurepip', 'enum', 'errno', 'exceptions', 'faulthandler', 'fcntl', 'filecmp', 'fileinput', 'findertools', 'fl', 'flp', 'fm', 'fnmatch', 'formatter', 'fpectl', 'fpformat', 'fractions', 'ftplib', 'functools', 'future_builtins', 'gc', 'gdbm', 'gensuitemodule', 'getopt', 'getpass', 'gettext', 'gl', 'glob', 'grp', 'gzip', 'hashlib', 'heapq', 'hmac', 'hotshot', 'html', 'htmlentitydefs', 'htmllib', 'http', 'httplib', 'ic', 'icopen', 'imageop', 'imaplib', 'imgfile', 'imghdr', 'imp', 'importlib', 'imputil', 'inspect', 'io', 'ipaddress', 'itertools', 'jpeg', 'json', 'keyword', 'lib2to3', 'linecache', 'locale', 'logging', 'lzma', 'macerrors', 'macostools', 'macpath', 'macresource', 'mailbox', 'mailcap', 'marshal', 'math', 'md5', 'mhlib', 'mimetools', 'mimetypes', 'mimify', 'mmap', 'modulefinder', 'msilib', 'msvcrt', 'multifile', 'multiprocessing', 'mutex', 'netrc', 'new', 'nis', 'nntplib', 'numbers', 'operator', 'optparse', 'os', 'ossaudiodev', 'parser', 'pathlib', 'pdb', 'pickle', 'pickletools', 'pipes', 'pkgutil', 'platform', 'plistlib', 'popen2', 'poplib', 'posix', 'posixfile', 'pprint', 'profile', 'pstats', 'pty', 'pwd', 'py_compile', 'pyclbr', 'pydoc', 'queue', 'quopri', 'random', 're', 'readline', 'reprlib', 'resource', 'rexec', 'rfc822', 'rlcompleter', 'robotparser', 'runpy', 'sched', 'secrets', 'select', 'selectors', 'sets', 'sgmllib', 'sha', 'shelve', 'shlex', 'shutil', 'signal', 'site', 'sitecustomize', 'smtpd', 'smtplib', 'sndhdr', 'socket', 'socketserver', 'spwd', 'sqlite3', 'ssl', 'stat', 'statistics', 'statvfs', 'string', 'stringprep', 'struct', 'subprocess', 'sunau', 'sunaudiodev', 'symbol', 'symtable', 'sys', 'sysconfig', 'syslog', 'tabnanny', 'tarfile', 'telnetlib', 'tempfile', 'termios', 'test', 'textwrap', 'this', 'thread', 'threading', 'time', 'timeit', 'tkinter', 'token', 'tokenize', 'trace', 'traceback', 'tracemalloc', 'ttk', 'tty', 'turtle', 'turtledemo', 'types', 'typing', 'unicodedata', 'unittest', 'urllib', 'urllib2', 'urlparse', 'usercustomize', 'uu', 'uuid', 'venv', 'videoreader', 'warnings', 'wave', 'weakref', 'webbrowser', 'whichdb', 'winreg', 'winsound', 'wsgiref', 'xdrlib', 'xml', 'xmlrpc', 'xmlrpclib', 'zipapp', 'zipfile', 'zipimport', 'zlib'], known_third_party=['google.appengine.api', 'py', 'pytest'], known_first_party=[], multi_line_output=3, forced_separate=[], indent= , comment_prefix= #, length_sort=False, add_imports=[], remove_imports=[], reverse_relative=False, force_single_line=False, default_section=FIRSTPARTY, import_heading_future=, import_heading_stdlib=, import_heading_thirdparty=, import_heading_firstparty=, import_heading_localfolder=, balanced_wrapping=False, use_parentheses=True, order_by_type=True, atomic=False, lines_after_imports=-1, lines_between_sections=1, lines_between_types=0, combine_as_imports=False, combine_star=False, keep_direct_and_as_imports=False, include_trailing_comma=True, from_first=False, verbose=False, quiet=False, force_adds=False, force_alphabetical_sort_within_sections=False, force_alphabetical_sort=False, force_grid_wrap=0, force_sort_within_sections=False, show_diff=False, ignore_whitespace=False, no_lines_before=[], no_inline_sort=False, ignore_comments=False, safety_excludes=True, case_sensitive=False)

Also strings are unquoted, so at least this would be appropriate:

Suggested change

", ".join(f"{k}={v}" for k, v in isort_settings.items())

", ".join(f"{k}={v!r}" for k, v in isort_settings.items())

I experimented with an abbreviation mechanism as well, adding this function:

def abbreviate(value): if isinstance(value, (tuple, list)) and len(value) > 2: return "..." return repr(value)

and using it here:

Suggested change

", ".join(f"{k}={v}" for k, v in isort_settings.items())

", ".join(f"{k}={abbreviate(v)}" for k, v in isort_settings.items())

With that, debug output shrinks to:

DEBUG:darker.import_sorting:SortImports(file_contents=..., check=True, force_to_top=[], skip=[], skip_glob=[], line_length=88, wrap_length=0, line_ending=None, sections=..., no_sections=False, known_future_library=['__future__'], known_standard_library=..., known_third_party=..., known_first_party=[], multi_line_output=3, forced_separate=[], indent=' ', comment_prefix=' #', length_sort=False, add_imports=[], remove_imports=[], reverse_relative=False, force_single_line=False, default_section='FIRSTPARTY', import_heading_future='', import_heading_stdlib='', import_heading_thirdparty='', import_heading_firstparty='', import_heading_localfolder='', balanced_wrapping=False, use_parentheses=True, order_by_type=True, atomic=False, lines_after_imports=-1, lines_between_sections=1, lines_between_types=0, combine_as_imports=False, combine_star=False, keep_direct_and_as_imports=False, include_trailing_comma=True, from_first=False, verbose=False, quiet=False, force_adds=False, force_alphabetical_sort_within_sections=False, force_alphabetical_sort=False, force_grid_wrap=0, force_sort_within_sections=False, show_diff=False, ignore_whitespace=False, no_lines_before=[], no_inline_sort=False, ignore_comments=False, safety_excludes=True, case_sensitive=False)

It's still quite a bit. Maybe we should replace all of those with a , ..., or if we want to go through the trouble, only include items which differ from the defaults. What's your opinion?

Co-authored-by: Antti Kaihola <13725+akaihola@users.noreply.github.com>

Mystic-Mirage

pytest-isort is broken since isort v5.0, see stephrdev/pytest-isort#23

Also as darker

There is no isort.SortImports anymore

…tings" This reverts commit 3e59c2c

Mystic-Mirage · 2020-07-06T01:28:19Z

I'm closing this in favor of #18

Mystic-Mirage force-pushed the isort_config branch 10 times, most recently from a35186f to 613b535 Compare July 3, 2020 00:24

Mystic-Mirage marked this pull request as ready for review July 3, 2020 00:26

Mystic-Mirage requested review from akaihola and CorreyL July 3, 2020 00:26

Mystic-Mirage force-pushed the isort_config branch 2 times, most recently from 988a2af to 77322d0 Compare July 3, 2020 17:38

Allow to configure isort through pyproject.toml

c7ecd66

Mystic-Mirage force-pushed the isort_config branch from 77322d0 to c7ecd66 Compare July 3, 2020 18:37

akaihola requested changes Jul 3, 2020

View reviewed changes

Mystic-Mirage and others added 3 commits July 4, 2020 14:55

Update CHANGES.rst

98c75c1

Co-authored-by: Antti Kaihola <13725+akaihola@users.noreply.github.com>

Remove unnecessary third party packages list from isort settings

3e59c2c

Pin isort version because isort >= 5 breaks everything

e8a5704

Mystic-Mirage commented Jul 4, 2020

View reviewed changes

Mystic-Mirage added 2 commits July 4, 2020 17:18

Revert "Remove unnecessary third party packages list from isort set…

0f352bc

…tings" This reverts commit 3e59c2c

Use isort.settings as a single module

659dc0c

akaihola mentioned this pull request Jul 4, 2020

Compatibility with isort 5.x #15

Closed

3 tasks

Mystic-Mirage mentioned this pull request Jul 4, 2020

Add support of isort>5 #16

Merged

1 task

Mystic-Mirage marked this pull request as draft July 5, 2020 20:33

Mystic-Mirage mentioned this pull request Jul 6, 2020

Allow to configure isort through pyproject.toml #18

Merged

Mystic-Mirage closed this Jul 6, 2020

Mystic-Mirage deleted the isort_config branch July 6, 2020 01:28

akaihola added enhancement New feature or request duplicate This issue or pull request already exists labels Jul 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to configure `isort` through pyproject.toml #13

Allow to configure `isort` through pyproject.toml #13

Mystic-Mirage commented Jul 2, 2020

akaihola commented Jul 3, 2020

Mystic-Mirage commented Jul 3, 2020

akaihola commented Jul 3, 2020 •

edited

Loading

akaihola commented Jul 3, 2020

Mystic-Mirage commented Jul 3, 2020

akaihola Jul 3, 2020 •

edited

Loading

akaihola Jul 3, 2020

akaihola Jul 3, 2020

akaihola Jul 3, 2020

akaihola Jul 3, 2020

akaihola Jul 3, 2020

akaihola Jul 3, 2020

akaihola Jul 3, 2020

akaihola Jul 3, 2020

akaihola Jul 3, 2020

akaihola Jul 3, 2020

Mystic-Mirage Jul 4, 2020

akaihola Jul 3, 2020

Mystic-Mirage left a comment

Mystic-Mirage commented Jul 6, 2020

	src: Path, src_contents: str, black_args: Dict[str, Union[bool, int, str]]
	src: Path, src_contents: str, black_args: BlackArgs

		args = cast(Dict[str, Union[bool, int]], black_args)
		combined_args = {defaults, args}

	args = cast(Dict[str, Union[bool, int]], black_args)
	combined_args = {defaults, args}
	combined_args = {defaults, black_args}

-from typing import Dict, List, Optional, Tuple, Union, cast
+import sys
+from typing import List, Optional, cast
+if sys.version_info >= (3, 8):
+    from typing import TypedDict
+else:
+    from typing_extensions import TypedDict

		from typing import Dict, List, Optional, Tuple, Union, cast

		from black import find_project_root


		logger = logging.getLogger(__name__)

		IsortSettings = Dict[str, Union[bool, int, List[str], str, Tuple[str], None]]

-IsortSettings = Dict[str, Union[bool, int, List[str], str, Tuple[str], None]]
+class IsortSettings(TypedDict):
+    line_length: int

	", ".join(f"{k}={v}" for k, v in isort_settings.items())
	", ".join(f"{k}={v!r}" for k, v in isort_settings.items())

	", ".join(f"{k}={v}" for k, v in isort_settings.items())
	", ".join(f"{k}={abbreviate(v)}" for k, v in isort_settings.items())

Allow to configure isort through pyproject.toml #13

Allow to configure isort through pyproject.toml #13

Conversation

Mystic-Mirage commented Jul 2, 2020

akaihola commented Jul 3, 2020

Mystic-Mirage commented Jul 3, 2020

akaihola commented Jul 3, 2020 • edited Loading

akaihola commented Jul 3, 2020

Mystic-Mirage commented Jul 3, 2020

akaihola Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mystic-Mirage left a comment

Choose a reason for hiding this comment

Mystic-Mirage commented Jul 6, 2020

Allow to configure `isort` through pyproject.toml #13

Allow to configure `isort` through pyproject.toml #13

akaihola commented Jul 3, 2020 •

edited

Loading

akaihola Jul 3, 2020 •

edited

Loading