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.

Sign up for GitHub

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

Ensure that split_on_trailing_comma works with as imports #2340

Merged
merged 5 commits into from
Jan 20, 2025
Merged

Ensure that split_on_trailing_comma works with as imports #2340

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e264241
Ensure that split_on_trailing_comma works with `as` imports
DanielNoord Jan 18, 2025
3110618
Also fix styling issues
DanielNoord Jan 18, 2025
a5d83cf
Ignore DeepSource
DanielNoord Jan 18, 2025
fa2abe9
Add correct ignore
DanielNoord Jan 18, 2025
b96c0ea
Add `xfail` for `django`
DanielNoord Jan 20, 2025
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
9 changes: 8 additions & 1 deletion isort/output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ def sorted_imports(
return _output_as_string(formatted_output, parsed.line_separator)


# Ignore DeepSource cyclomatic complexity check for this function. It was
# already complex when this check was enabled.
# skipcq: PY-R1000
def _with_from_imports(
parsed: parse.ParsedContent,
config: Config,
Expand Down Expand Up @@ -509,7 +512,11 @@ def _with_from_imports(
):
do_multiline_reformat = True

if config.split_on_trailing_comma and module in parsed.trailing_commas:
if (
import_statement
and config.split_on_trailing_comma
and module in parsed.trailing_commas
):
import_statement = wrap.import_statement(
import_start=import_start,
from_imports=from_import_section,
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/profiles/test_black.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,8 +447,8 @@ def sub(a: np.ndarray, b: np.ndarray) -> np.ndarray: ...

def test_black_trailing_comma():
black_test(
"from x import (a, b, c,)\n",
"""\
"from x import (a, b, c,)\n",
"""\
from x import (
a,
b,
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/test_isort.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5576,6 +5576,18 @@ def test_split_on_trailing_comma() -> None:
assert output == expected_output


def test_split_on_trailing_comma_wih_as() -> None:
test_input = "from lib import (a as b,)"
expected_output = """from lib import a as b
"""
Comment on lines +5580 to +5582
Copy link
Contributor

@matthewhughes934 matthewhughes934 Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

what's expected for multiple as imports an a trailing comma, e.g. from lib import (a as b, c as d,)?

from lib import a as b
from lib import c as d

or

from lib import (
    a as b,
    c as d,
)

Copy link
Member Author

Choose a reason for hiding this comment

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

Current code will do:

from lib import a as b
from lib import c as d


output = isort.code(test_input, split_on_trailing_comma=True)
assert output == expected_output

output = isort.code(expected_output, split_on_trailing_comma=True)
assert output == expected_output


def test_infinite_loop_in_unmatched_parenthesis() -> None:
test_input = "from os import ("

Expand Down
Loading