Skip to content

Keep overlong star imports stable with Black - #2652

Closed
Spidy092 wants to merge 5 commits into
PyCQA:mainfrom
Spidy092:fix/keep-star-imports-stable
Closed

Spidy092 wants to merge 5 commits into
PyCQA:mainfrom
Spidy092:fix/keep-star-imports-stable

Conversation

@Spidy092

@Spidy092 Spidy092 commented Sep 4, 2026

Copy link
Copy Markdown

Summary

  • Keep over-long from ... import * and from ... cimport * statements intact when they cannot be safely wrapped.
  • Preserve trailing comments and the existing NOQA wrapping mode behavior.
  • Add regression coverage for direct wrapping and the Black profile.

Fixes #2649

Testing

  • python3 -m pytest -q tests/unit/test_wrap.py (8 passed)
  • ruff check isort/wrap.py tests/unit/test_wrap.py
  • ruff format --check isort/wrap.py tests/unit/test_wrap.py
  • Direct reproduction: output remains unchanged, compiles, and is idempotent under profile="black".
  • The full tests/unit collection is blocked in the isolated harness because the test-only dependencies black, hypothesis, and colorama are unavailable.

@Manny7717 Manny7717 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified at head 9ef16a1 — this resolves the isort↔black non-convergence from #2649 exactly as reported.

Regression-proven: the 3 rewritten star-import spec tests FAIL on base 131f4ad (backslash output still emitted: import \\ + *) and PASS at head (statement returned untouched). 8/8 in tests/unit/test_wrap.py at head.

End-to-end convergence probe (real CLI, black 26.3.1, profile=black, over-long from ... import *):

  • Base 131f4ad: isort backslash-wraps → black rejoins to one line → isort wraps again → alternates forever (no fixed point — reproduces #2649 verbatim, incl. "Fixing" on every isort pass).
  • Head: isort leaves the line → black leaves it → isort leaves it. Stable after one pass.

No collateral: full tests/unit at head 623 passed / 2 failed / 1 skipped — the 2 failures are the pre-existing issue_909/issue_938 FileNotFoundError env-noise tests (identical on clean main across all my prior runs). The endswith("import *") condition is stricter than the old endswith("*") (only star from/cimport lines match; short star imports still return via the length check before this branch; comments preserved via the earlier # split). The #2267 corruption fix (never emit import (*)) remains satisfied — the statement is now kept whole and valid instead of backslash-split.

Non-blocking nit: the # skipcq: PY-R1000 directive comments a function whose complexity this PR actually reduces (branch removed, ~6 lines deleted); the two-line comment is also redundant. If the DeepSource gate still trips at the threshold, fine to keep, but consider dropping both if the removal alone clears it — external-service gates shouldn't pin unrelated comments in the tree.

Comment thread tests/unit/test_wrap.py

See issue #2267.
"""
def test_line_star_import_is_not_wrapped() -> None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not using the black profile. Why are we changing behaviour here?

@codecov

codecov Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.37%. Comparing base (131f4ad) to head (9ef16a1).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2652      +/-   ##
==========================================
- Coverage   99.37%   99.37%   -0.01%     
==========================================
  Files          41       41              
  Lines        3181     3179       -2     
  Branches      686      686              
==========================================
- Hits         3161     3159       -2     
  Misses         12       12              
  Partials        8        8              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@DanielNoord

Copy link
Copy Markdown
Member

Superseded by #2665 which is a more targetted fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isort disagrees with black on wrapping over-long star imports

3 participants