Skip to content

Keep overlong star imports stable with Black - #2652

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

Keep overlong star imports stable with Black#2652
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

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.

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

2 participants