Sorting order is broken and not following standard sort order

[inboxcda]

I love the idea of isort! Genius! However, I know the recommended way to sort in Python is something like this:

On Top: standard libraries Middle: third party libraries Bottom: packages/files defined locally (in the tree where this files resides)

This doesn't seem to be the case with isort and I cannot find hardly anything online about it. Sometimes the lines are not even sorted alphabetically, and also it doesn't seem to differentiate between pip packages and local packages as my pip packages are at the bottom most times. I have tried testing this on several files and projects on different computers with different VScode configurations and even clean vscode installs, and I don't really see the pattern working to the Python standard. It is possible to configure isort so that it does sort to the python standard?

I have tried with the autopep8 and black formatters. With the recommended json settings config. The sort happens, but it is sorting incorrectly to my understanding.

Thanks in advance for the help! I am really hoping to get this sort working properly or to be educated by someone if I am completely missing something.

Cheers, Chase

[bp72]

Hi Chase,

Thank you for the message! Would you mind providing me with an example, so I can try and replicate the issue you are experiencing? That would be super helpful!

[Felixoid]

Hi @bp72.

I am facing the same issue in https://github.com/ClickHouse/ClickHouse/pull/60444

You can quickly clone the ClickHouse/ClickHouse repository and jump across the tests/ci directory. There are a lot of hacky sort on/off comments, so every such place is pretty much an example of broken sorting.

[Felixoid]

Here's how to reproduce the issue:

$ git clone --filter=tree:0 git@github.com:ClickHouse/ClickHouse.git
$ cd ClickHouse/tests/ci
$ sed '/isort/d' -i pr_info.py
$ git diff
diff --git a/tests/ci/pr_info.py b/tests/ci/pr_info.py
index 6f4b400f7..cab038043 100644
--- a/tests/ci/pr_info.py
+++ b/tests/ci/pr_info.py
@@ -6,11 +6,9 @@ import re
 from typing import Dict, List, Set, Union
 from urllib.parse import quote

-# isort: off
 # for some reason this line moves to the end
 from unidiff import PatchSet  # type: ignore

-# isort: on

 from build_download_helper import get_gh_api
 from env_helper import (
$ isort -vvv pr_info.py

                 _                 _
                (_) ___  ___  _ __| |_
                | |/ _/ / _ \/ '__  _/
                | |\__ \/\_\/| |  | |_
                |_|\___/\___/\_/   \_/

      isort your imports, so you don't have to.

                    VERSION 5.13.2

else-type place_module for json returned STDLIB
else-type place_module for logging returned STDLIB
else-type place_module for os returned STDLIB
else-type place_module for re returned STDLIB
from-type place_module for typing returned STDLIB
from-type place_module for urllib.parse returned STDLIB
from-type place_module for unidiff returned THIRDPARTY
from-type place_module for build_download_helper returned THIRDPARTY
from-type place_module for env_helper returned THIRDPARTY
Fixing /tmp/ClickHouse/tests/ci/pr_info.py
$ git diff
diff --git a/tests/ci/pr_info.py b/tests/ci/pr_info.py
index 6f4b400f7..09b329aa4 100644
--- a/tests/ci/pr_info.py
+++ b/tests/ci/pr_info.py
@@ -6,20 +6,12 @@ import re
 from typing import Dict, List, Set, Union
 from urllib.parse import quote

-# isort: off
+from build_download_helper import get_gh_api
+from env_helper import (GITHUB_EVENT_PATH, GITHUB_REPOSITORY, GITHUB_RUN_URL,
+                        GITHUB_SERVER_URL)
 # for some reason this line moves to the end
 from unidiff import PatchSet  # type: ignore

-# isort: on
-
-from build_download_helper import get_gh_api
-from env_helper import (
-    GITHUB_EVENT_PATH,
-    GITHUB_REPOSITORY,
-    GITHUB_RUN_URL,
-    GITHUB_SERVER_URL,
-)
-
 SKIP_MERGEABLE_CHECK_LABEL = "skip mergeable check"
 NeedsDataType = Dict[str, Dict[str, Union[str, Dict[str, str]]]]
$ python -c 'import unidiff; print(unidiff.__file__)'
/usr/lib/python3.11/site-packages/unidiff/__init__.py

[Felixoid]

It looks like the issue is the default config.src_paths

# default
(Pdb) p config.src_paths
(PosixPath('/tmp/ClickHouse/src'), PosixPath('/tmp/ClickHouse'))

# with --src .
(Pdb) p config.src_paths
(PosixPath('/tmp/ClickHouse/tests/ci'),)

$ git checkout .
$ sed '/isort/d' -i pr_info.py
$ isort -vvvvvvv pr_info.py --interactive --src . --profile=black

                 _                 _
                (_) ___  ___  _ __| |_
                | |/ _/ / _ \/ '__  _/
                | |\__ \/\_\/| |  | |_
                |_|\___/\___/\_/   \_/

      isort your imports, so you don't have to.

                    VERSION 5.13.2

else-type place_module for json returned STDLIB
else-type place_module for logging returned STDLIB
else-type place_module for os returned STDLIB
else-type place_module for re returned STDLIB
from-type place_module for typing returned STDLIB
from-type place_module for urllib.parse returned STDLIB
from-type place_module for unidiff returned THIRDPARTY
from-type place_module for build_download_helper returned FIRSTPARTY
from-type place_module for env_helper returned FIRSTPARTY
--- /tmp/ClickHouse/tests/ci/pr_info.py:before  2024-03-13 15:30:51.930443
+++ /tmp/ClickHouse/tests/ci/pr_info.py:after   2024-03-13 15:30:55.829464
@@ -8,7 +8,6 @@

 # for some reason this line moves to the end
 from unidiff import PatchSet  # type: ignore
-

 from build_download_helper import get_gh_api
 from env_helper import (
Apply suggested changes to '/tmp/ClickHouse/tests/ci/pr_info.py' [y/n/q]? y
Fixing /tmp/ClickHouse/tests/ci/pr_info.py
$ git diff
diff --git a/tests/ci/pr_info.py b/tests/ci/pr_info.py
index 6f4b400f7..b9dd6684d 100644
--- a/tests/ci/pr_info.py
+++ b/tests/ci/pr_info.py
@@ -6,12 +6,9 @@ import re
 from typing import Dict, List, Set, Union
 from urllib.parse import quote

-# isort: off
 # for some reason this line moves to the end
 from unidiff import PatchSet  # type: ignore

-# isort: on
-
 from build_download_helper import get_gh_api
 from env_helper import (
     GITHUB_EVENT_PATH,

So, here's the question. Is there a way to define all known first-party directories in the repository?

update: pylint actually understands, that this is a first-party library:

> pylint pr_info.py
************* Module pr_info
pr_info.py:18:0: C0411: third party import "unidiff.PatchSet" should be placed before first party imports "build_download_helper.get_gh_api", "env_helper.GITHUB_EVENT_PATH"  (wrong-import-order)

[Felixoid]

The issue can't be solved in nice and easy way.

module_with_reason is not aware of the file path.

I think I can try to change its signature (and all others) to accept an optional src_path based on the file name or the current directory, if the input is stdin, and pass it to file_contents and all down the road. WDYT?

[Felixoid]

Dear @bp72 and @staticdev, I'd like to include isort into our CI, but this issue makes it hard to use. Either # isort: off should be used or a complex configuration.

I want to fix it myself, but it's helpful to discuss how to do it correctly in advance.

[sztamas]

Hi @Felixoid ,

Can't you just add your tests to src_paths?

In your pyproject.toml:

[tool.isort]
profile = "black"
src_paths = ["src", "tests/ci"]

Continuing your example :arrow_up: and configuring using pyproject.toml.

❯ git -P diff
diff --git a/pyproject.toml b/pyproject.toml
index 3d05abd9e..68d22f973 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -38,3 +38,6 @@ disable = '''
 # due to SQL
 min-similarity-lines=1000

+[tool.isort]
+profile = "black"
+src_paths = ["src", "tests/ci"]
diff --git a/tests/ci/pr_info.py b/tests/ci/pr_info.py
index c61e62f33..2cb83a22d 100644
--- a/tests/ci/pr_info.py
+++ b/tests/ci/pr_info.py
@@ -6,12 +6,9 @@ import re
 from typing import Dict, List, Set, Union
 from urllib.parse import quote

-# isort: off
 # for some reason this line moves to the end
 from unidiff import PatchSet  # type: ignore

-# isort: on
-
 from build_download_helper import get_gh_api
 from env_helper import (
     GITHUB_EVENT_PATH,

❯ isort -v tests/ci/pr_info.py

                 _                 _
                (_) ___  ___  _ __| |_
                | |/ _/ / _ \/ '__  _/
                | |\__ \/\_\/| |  | |_
                |_|\___/\___/\_/   \_/

      isort your imports, so you don't have to.

                    VERSION 5.13.2

else-type place_module for json returned STDLIB
else-type place_module for logging returned STDLIB
else-type place_module for os returned STDLIB
else-type place_module for re returned STDLIB
from-type place_module for typing returned STDLIB
from-type place_module for urllib.parse returned STDLIB
from-type place_module for unidiff returned THIRDPARTY
from-type place_module for build_download_helper returned FIRSTPARTY
from-type place_module for env_helper returned FIRSTPARTY

❯ git -P diff tests/ci/pr_info.py
diff --git a/tests/ci/pr_info.py b/tests/ci/pr_info.py
index c61e62f33..2cb83a22d 100644
--- a/tests/ci/pr_info.py
+++ b/tests/ci/pr_info.py
@@ -6,12 +6,9 @@ import re
 from typing import Dict, List, Set, Union
 from urllib.parse import quote

-# isort: off
 # for some reason this line moves to the end
 from unidiff import PatchSet  # type: ignore

-# isort: on
-
 from build_download_helper import get_gh_api
 from env_helper import (
     GITHUB_EVENT_PATH,

[Felixoid]

@sztamas interesting. I didn't know it worked as well when the isort is executed from any place in the repository. E.g.:

> git diff
diff --git a/pyproject.toml b/pyproject.toml
index 3d05abd9ec2..ab21d3064cb 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -7,6 +7,10 @@ max-branches=50
 max-nested-blocks=10
 max-statements=200

+[tool.isort]
+profile = "black"
+src_paths = ["src", "tests/ci"]
+
 [tool.pylint.FORMAT]
 #ignore-long-lines = (# )?<?https?://\S+>?$

diff --git a/tests/ci/pr_info.py b/tests/ci/pr_info.py
index c61e62f334c..f134487c0a2 100644
--- a/tests/ci/pr_info.py
+++ b/tests/ci/pr_info.py
@@ -6,12 +6,8 @@ import re
 from typing import Dict, List, Set, Union
 from urllib.parse import quote

-# isort: off
-# for some reason this line moves to the end
 from unidiff import PatchSet  # type: ignore

-# isort: on
-
 from build_download_helper import get_gh_api
 from env_helper import (
     GITHUB_EVENT_PATH,

> cd tests
> isort ci/pr_info.py -v

                 _                 _
                (_) ___  ___  _ __| |_
                | |/ _/ / _ \/ '__  _/
                | |\__ \/\_\/| |  | |_
                |_|\___/\___/\_/   \_/

      isort your imports, so you don't have to.

                    VERSION 5.13.2

else-type place_module for json returned STDLIB
else-type place_module for logging returned STDLIB
else-type place_module for os returned STDLIB
else-type place_module for re returned STDLIB
from-type place_module for typing returned STDLIB
from-type place_module for urllib.parse returned STDLIB
from-type place_module for unidiff returned THIRDPARTY
from-type place_module for build_download_helper returned FIRSTPARTY
from-type place_module for env_helper returned FIRSTPARTY
> cd ci/lambda_shared_package
> isort ../pr_info.py -v

                 _                 _
                (_) ___  ___  _ __| |_
                | |/ _/ / _ \/ '__  _/
                | |\__ \/\_\/| |  | |_
                |_|\___/\___/\_/   \_/

      isort your imports, so you don't have to.

                    VERSION 5.13.2

else-type place_module for json returned STDLIB
else-type place_module for logging returned STDLIB
else-type place_module for os returned STDLIB
else-type place_module for re returned STDLIB
from-type place_module for typing returned STDLIB
from-type place_module for urllib.parse returned STDLIB
from-type place_module for unidiff returned THIRDPARTY
from-type place_module for build_download_helper returned FIRSTPARTY
from-type place_module for env_helper returned FIRSTPARTY

I assume it's enough to add any problematic path to the pyproject? It looks relatively easy fix for our case.

[sztamas]

Yes, you should add all directories where your project stores source files to src_paths. That's how isort will know about your project structure and what it should consider FIRSTPARTY.

I hope this helps!

[Felixoid]

It does indeed, thank you!

Would you consider improvements/brainstorming on how to identify it automatically?

[sztamas]

No worries, I'm glad it helped!

To be honest, I wasn't active on this project for quite a while, so it would quite an effort and I'm not sure if I'll have the bandwidth for it. Automatic determination of src_paths has been tried before by @timothycrosley , but it was rolled back because of inconsistent behavior.

1239 and the linked issues/commits are a good read if you're interested in the details.

[Felixoid]

hmmmmmm... I see. The issue clarifies the context very well. Maybe the project settings work better than magic.

[sztamas]

Hi @inboxcda

I know that it's been a while, but , as the OP, if you could have any other example when isort doesn't behave as you expect it, could you please provide it?

Based on the problems you are seeing, it might be the same issue above. ie. src_paths not configured and therefore isort would identify you modules as THIRDPARTY instead of FIRSTPARTY.

Thanks, Tamas