black: Unexpected incompatibility with reorder-python-imports

Describe the bug

It is impossible to use Black 24.1.0 alongside reorder-python-imports. Using the example from #1872 , reorder-python-imports produces:

"""Module docstring."""
import typing 

While formatting with black and default arguments produces the following:

"""Module docstring."""

import typing

This new behaviour makes the two tools incompatible. The author of reorder-python-imports suggests this is an issue with Black.

Is it possible to change Black to work better with reorder-python-imports please? I have used both tools together for a long time.

Environment

black, 24.1.0 (compiled: yes)
Python (CPython) 3.12.1

Also checked on the online formatter.

Additional context

A little bit of context from the Black changelog is that Black 24 and above will:

Enforce newline after module docstrings (#3932, #4028)

1872 was the original Black issue, and changes were implemented in 3932; 4028 doesn’t relate to imports.

About this issue

  • Original URL
  • State: open
  • Created 5 months ago
  • Reactions: 6
  • Comments: 15 (3 by maintainers)

Commits related to this issue

Most upvoted comments

maybe just use isort with something like:

[tool.isort]
profile = "black"
force_single_line = "true"

I’ve decided to migrate my projects to isort with the configuration:

[tool.isort]
add_imports = [
    "from __future__ import annotations"
]
force_single_line = true
profile = "black"

isort is still sufficiently fast for me and it does have a goal of supporting Black.

The guy who maintains reorder-python-imports is very “my way or the highway”, I suggest you take that advice and use isort or the fork suggestion @adamchainz made. The place I work already dropped reorder-python-imports not because it didn’t work for us but specifically because of his attitude. If one of you does fork it please let me know, we would definitely consider switching to an import sorter that had the same sort of considered opination that Black has.

Might be worth asking Ruff to implement the one-import-per-line style as an option. (I searched their issue tracker but didn’t find an existing issue asking for it.)

It’s unfortunate that nobody reported this while the feature was in the preview style and in the alphas. Our guarantee is that we’ll keep the style the same for the rest of the year, so we can’t change it now.

I think it should be Black’s job to decide on whitespace outside the import block, not reorder-python-imports’s. Unfortunately that means I don’t have a good resolution for you.

🤷 This is a real shame, I really like the speed and low-configurability of reorder-python-imports. I do also think its rearrangement of non-import newlines is outside its wheelhouse.

FWIW, below is the small patch that makes reorder-python-imports compatible. Debating whether it’s worth forking to make an “always Black-compatible” version…

diff --git reorder_python_imports.py reorder_python_imports.py
index d66dc4b..3de766f 100644
--- reorder_python_imports.py
+++ reorder_python_imports.py
@@ -94,10 +94,10 @@ def partition_source(src: str) -> tuple[str, list[str], str, str]:
             pre_import = False
             chunks.append((CodeType.IMPORT, s))
         elif token_type is Tok.NEWLINE:
-            if s.isspace():
-                tp = CodeType.NON_CODE
-            elif pre_import:
+            if pre_import:
                 tp = CodeType.PRE_IMPORT_CODE
+            elif s.isspace():
+                tp = CodeType.NON_CODE
             else:
                 tp = CodeType.CODE
 
diff --git tests/reorder_python_imports_test.py tests/reorder_python_imports_test.py
index c61c39a..4db72fd 100644
--- tests/reorder_python_imports_test.py
+++ tests/reorder_python_imports_test.py
@@ -47,7 +47,7 @@ def test_tokenize_can_match_strings(s):
 @pytest.mark.parametrize(
     's',
     (
-        pytest.param('', id='trivial'),
+        pytest.param('\n', id='trivial'),
         pytest.param('#!/usr/bin/env python\n', id='shebang'),
         pytest.param('# -*- coding: UTF-8 -*-\n', id='source encoding'),
         pytest.param('  # coding: UTF-8\n', id='source encoding indented'),
@@ -190,17 +190,21 @@ def test_partition_source_imports_only(s, expected):
     assert nl == '\n'
 
 
-def test_partition_source_before_removes_newlines():
+def test_partition_source_before_leaves_newlines():
     before, imports, after, nl = partition_source(
         '# comment here\n'
         '\n'
-        '# another comment here\n',
+        '# another comment here\n'
+        '\n'
+        'import os\n'
     )
     assert before == (
         '# comment here\n'
+        '\n'
         '# another comment here\n'
+        '\n'
     )
-    assert imports == []
+    assert imports == ['import os\n']
     assert after == ''
     assert nl == '\n'
 

I suggest for this issue to be closed, with the solution being using isort: https://github.com/psf/black/issues/4175#issuecomment-1936708759

Perhaps it can be pinned for some time so others can find it quickly. 👍

The only reason to change that I am aware of is to make Black compatible with reorder-python-imports again. Is that a good reason or not? I’ll let you decide.

I agree both are opinionated tools with a specific style. I should perhaps make it explicit here that the style implemented by reorder-python-imports has “a single aim: reduce merge conflicts” with a documented rationale. A short example is:

-from typing import Dict, List
+from typing import Dict
+from typing import List

The other functionality in reorder-python-imports is also available in other import sorting tools. I looked at isort and ruff; usort is new to me, thank you for mentioning it. I am not aware of a tool other than reorder-python-imports that implements this “reduce merge conflicts” import style.

isort is the only documented compatible import ordering tool for black. Does that imply anything about future compatibilty?

The ignore comments ( # fmt: skip after the closing “”" ) I mentioned above are OK for a short term workaround. Longer term, if neither tool is going to change, you’re right I’ll need to carefully reconsider my tool choices.

Edited to add: thanks again for dealing with this unfortunate incompatibility carefully. I appreciate your help.