Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support of isort>5 #16

Merged
merged 4 commits into from
Jul 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- Feature: Add support for black config
- Feature: Add support for ``-l``/``--line-length`` and ``-S``/``--skip-string-normalization``
- Feature: ``--diff`` outputs a diff for each file on standard output
- Feature: Require ``isort`` >= 5.0.1 and be compatible with it.


0.2.0 / 2020-03-11
Expand Down
3 changes: 2 additions & 1 deletion pytest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ doctest_optionflags = NORMALIZE_WHITESPACE
show_capture = no
addopts =
--black
--isort
## Disable pytest-isort until it is fixed to work with isort 5.x
# --isort
--mypy
--doctest-modules
5 changes: 3 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ console_scripts =

[options.extras_require]
isort =
isort
isort>=5.0.1
test =
pytest
pytest-black
pytest-isort
## Disable pytest-isort until it is fixed to work with isort 5.x
#pytest-isort
pytest-mypy
4 changes: 2 additions & 2 deletions src/darker/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
opcodes_to_edit_linenums,
)
from darker.git import git_diff_name_only, git_get_unmodified_content
from darker.import_sorting import SortImports, apply_isort
from darker.import_sorting import apply_isort, isort
from darker.utils import get_common_root, joinlines
from darker.verification import NotEquivalentError, verify_ast_unchanged
from darker.version import __version__
Expand Down Expand Up @@ -172,7 +172,7 @@ def main(argv: List[str] = None) -> None:
if args.version:
print(__version__)

if args.isort and not SortImports:
if args.isort and not isort:
logger.error(f"{ISORT_INSTRUCTION} to use the `--isort` option.")
exit(1)

Expand Down
24 changes: 11 additions & 13 deletions src/darker/import_sorting.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,27 @@
import logging
from pathlib import Path
from typing import List, cast

try:
from isort import SortImports
import isort
except ImportError:
SortImports = None
isort = None

logger = logging.getLogger(__name__)


def apply_isort(content: str) -> str:
logger.debug(
"SortImports(file_contents=..., check=True, multi_line_output=3, "
"include_trailing_comma=True, force_grid_wrap=0, use_parentheses=True, "
"line_length=88, quiet=True)"
)
result = SortImports(
file_contents=content,
check=True,
isort.check_code(code=content)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this call needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replaces check=True and writes error messages to stdout. See changes in test_main.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the error messages?

And if we do, is there a cleaner way to capture them than stdout?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the error messages?

I don't know... Some tests depend on error messages. Probably, we could raise an exception if check_code is False

And if we do, is there a cleaner way to capture them than stdout?

Hm... isort has hardcoded prints to write to stdout

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I had to include those error messages in the test assertions, because in isort<5 there was no way to get rid of the messages.

Now that we can run isort without polluting stdout with error messages, I think we should do that for simplicity's sake, and fix the tests as well – unless we get some benefit from having those error messages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought check=False in isort<5 was for this?
I vote to get rid of error messages in another PR in order to keep this PR clear.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vote to get rid of error messages in another PR in order to keep this PR clear.

I created another PR: #17

isort_config_kwargs = dict(
multi_line_output=3,
include_trailing_comma=True,
force_grid_wrap=0,
use_parentheses=True,
line_length=88,
quiet=True,
)
return cast(str, result.output)
logger.debug(
"isort.code(code=..., {})".format(
", ".join(f"{k}={v!r}" for k, v in isort_config_kwargs.items())
)
)
result: str = isort.code(code=content, **isort_config_kwargs)
return result
16 changes: 8 additions & 8 deletions src/darker/tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

def test_isort_option_without_isort(tmpdir, without_isort, caplog):
check_call(["git", "init"], cwd=tmpdir)
with patch.object(darker.__main__, "SortImports", None), pytest.raises(SystemExit):
with patch.object(darker.__main__, "isort", None), pytest.raises(SystemExit):

darker.__main__.main(["--isort", str(tmpdir)])

Expand All @@ -28,10 +28,10 @@ def run_isort(git_repo, monkeypatch, caplog):
paths['test1.py'].write('changed')
with patch.multiple(
darker.__main__, run_black=Mock(return_value=[]), verify_ast_unchanged=Mock(),
), patch("darker.import_sorting.SortImports"):
), patch("darker.import_sorting.isort.code"):
darker.__main__.main(["--isort", "./test1.py"])
return SimpleNamespace(
SortImports=darker.import_sorting.SortImports, caplog=caplog
isort_code=darker.import_sorting.isort.code, caplog=caplog
)


Expand All @@ -40,9 +40,8 @@ def test_isort_option_with_isort(run_isort):


def test_isort_option_with_isort_calls_sortimports(run_isort):
run_isort.SortImports.assert_called_once_with(
file_contents="changed",
check=True,
run_isort.isort_code.assert_called_once_with(
code="changed",
force_grid_wrap=0,
include_trailing_comma=True,
line_length=88,
Expand Down Expand Up @@ -111,7 +110,7 @@ def test_format_edited_parts_empty():
True,
{},
False,
['ERROR: Imports are incorrectly sorted.', ''],
['ERROR: Imports are incorrectly sorted and/or formatted.', ''],
A_PY_BLACK_ISORT,
),
(
Expand All @@ -126,7 +125,8 @@ def test_format_edited_parts_empty():
True,
{},
True,
['ERROR: Imports are incorrectly sorted.'] + A_PY_DIFF_BLACK_ISORT,
['ERROR: Imports are incorrectly sorted and/or formatted.']
+ A_PY_DIFF_BLACK_ISORT,
A_PY,
),
],
Expand Down