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.

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

# isort: split ignored if in trailing comment #11776

Open
ndevenish opened this issue Jun 6, 2024 · 5 comments
Open

# isort: split ignored if in trailing comment #11776

ndevenish opened this issue Jun 6, 2024 · 5 comments
Labels
isort Related to import sorting

Comments

@ndevenish
Copy link

test.py:

import x
import y

import f # isort: split

import a
import b

isort is a noop:

% isort --version
                 _                 _
                (_) ___  ___  _ __| |_
                | |/ _/ / _ \/ '__  _/
                | |\__ \/\_\/| |  | |_
                |_|\___/\___/\_/   \_/

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

                    VERSION 5.13.2
% isort --diff test.py
%

ruff ignores the marker:

% ruff --version
ruff 0.4.8
% ruff check --select=I --diff test.py
--- test.py
+++ test.py
@@ -1,8 +1,7 @@

-import x
-import y
-
-import f # isort: split
+import f  # isort: split

 import a
 import b
+import x
+import y

Would fix 3 errors.
@MichaReiser MichaReiser added the isort Related to import sorting label Jun 6, 2024
ndevenish added a commit to ndevenish/dials-fork that referenced this issue Jun 6, 2024
ndevenish added a commit to ndevenish/dials-fork that referenced this issue Jun 6, 2024
- Remove now-invalid noqa from pyslip
- Fix noqa in bootstrap.py
- Fix extra F403 wildcard imports
- Fix isinstance checks
- Fix E402 instances
- Fix "# isort: split" for ruff bug astral-sh/ruff#11776
ndevenish added a commit to ndevenish/dials-fork that referenced this issue Jun 6, 2024
- Remove now-invalid noqa from pyslip
- Fix noqa in bootstrap.py
- Fix extra F403 wildcard imports
- Fix isinstance checks
- Fix E402 instances
- Fix "# isort: split" for ruff bug astral-sh/ruff#11776
@charliermarsh
Copy link
Member

I find it a bit confusing. How should the import f be treated in such a case? Is it "before" or "after" the split, or neither?

@charliermarsh
Copy link
Member

I'm tempted to call this unsupported. Do you have a use-case for the inline split?

@ndevenish
Copy link
Author

ndevenish commented Jun 7, 2024

I find it a bit confusing. How should the import f be treated in such a case? Is it "before" or "after" the split, or neither?

It's effectively treated as

import x
import y

# isort: split
import f
# isort: split

import a
import b

This is explicitly documented in isort: https://pycqa.github.io/isort/docs/configuration/action_comments.html#isort-split

You can also use it inline to keep an import from having imports above or below it swap position:


I'm tempted to call this unsupported. Do you have a use-case for the inline split?

We've used this in the past for cases where upstream dependencies have not handled arbitrary-import-order very well, and so a specific import needs to be "First". We can work around by using normal split/on/off features.

Perhaps the fact that we're the only ones to notice this in so long is perhaps evidence that supporting it isn't very important.

@seproDev
Copy link

I am not sure if this is 100% related, but we also ran in to differences with trailing # isort: split. This is a noop in isort 5.13.2:

from x import a  # isort: split
from x import b

While ruff 0.4.8 changes it to:

from x import a  # isort: split

from x import b

When running

ruff check --select I --fix --isolated

@charliermarsh
Copy link
Member

Yeah I think we just don't really support inline # isort: split right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
isort Related to import sorting
Projects
None yet
Development

No branches or pull requests

4 participants