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 sections no longer respected? #11047

Closed
Badg opened this issue Apr 19, 2024 · 5 comments · Fixed by #11050
Closed

Isort sections no longer respected? #11047

Badg opened this issue Apr 19, 2024 · 5 comments · Fixed by #11050
Assignees
Labels
isort Related to import sorting question Asking for support or clarification

Comments

@Badg
Copy link

Badg commented Apr 19, 2024

I've noticed what I think has to be a bug/regression in ruff's implementation of isort grouping. Imports that were previously reported correct by ruff are now being reported as unsorted, in a way that conflicts with pyproject.toml. This is specific to import sections.

Some search keywords:

  • I001
  • import sorting
  • import groups
  • isort sections

Current ruff version is the package control 2024.04.19 release of LSP-ruff in sublime, which is, AFAIK, using the newest version of ruff.

In pyproject.toml, I have this configured (omitting the non-relevant stuff):

[tool.ruff.lint.isort]
force-single-line = true

[tool.ruff.lint.isort.sections]
testdeps = [
    "tt_website_testutils",
    "tt_website_tevecs",
    "tevec",
    "taev_ams_tevecs",]

However, the LSP linter incorrectly reports these as not sorted:

import tempfile
from pathlib import Path
from unittest.mock import Mock

import pytest

from contextual_singleton import Singleton
from contextual_singleton import apply_context
from taev_ams.collections.gitqlite import GitqliteAssetCollection
from taev_ams.collections.gitqlite import GitqliteSqlite
from taev_ams.collections.gitqlite import add_revision
from taev_ams.registry.registry_ import AssetRegistry
from taev_ams.tasks.collection_mgmt.create_collection import create_database

from taev_ams_tevecs.jinja_assets import LocalizedJinjaAsset
from taev_ams_tevecs.sass_assets import SassAsset
from taev_ams_tevecs.static_assets import FakeFontResource
from taev_ams_tevecs.static_assets import LocalizedStaticAsset
from taev_ams_tevecs.static_assets import SimpleStaticAsset
from taev_ams_tevecs.static_assets import SimpleStaticAssetV2

When I ask it to organize imports (via code actions), it incorrectly merges taev_ams_tevecs with the third-party dependencies:

import tempfile
from pathlib import Path
from unittest.mock import Mock

import pytest
from taev_ams_tevecs.jinja_assets import LocalizedJinjaAsset
from taev_ams_tevecs.sass_assets import SassAsset
from taev_ams_tevecs.static_assets import FakeFontResource
from taev_ams_tevecs.static_assets import LocalizedStaticAsset
from taev_ams_tevecs.static_assets import SimpleStaticAsset
from taev_ams_tevecs.static_assets import SimpleStaticAssetV2

from contextual_singleton import Singleton
from contextual_singleton import apply_context
from taev_ams.collections.gitqlite import GitqliteAssetCollection
from taev_ams.collections.gitqlite import GitqliteSqlite
from taev_ams.collections.gitqlite import add_revision
from taev_ams.registry.registry_ import AssetRegistry
from taev_ams.tasks.collection_mgmt.create_collection import create_database

I'm... not going crazy here, am I? I'm 99% sure I had ruff sorting these correctly, but I first noticed them being "wrong" ... yesterday? the day before yesterday? I'm not sure. Suspiciously close to the latest release, but memory is a fickle thing.

@charliermarsh
Copy link
Member

I think in #10149, the behavior was changed such that sections that aren't present in the sections setting are mapped to the default section, which is third-party? So now the imports in testdeps are being grouped with third-party.

You might want to add something like:

[tool.ruff.lint.isort.sections]
section-order = [
    "future",
    "third-party",
    "first-party",
    "local-folder",
    "testdeps",
]

@charliermarsh charliermarsh added question Asking for support or clarification isort Related to import sorting labels Apr 19, 2024
@Badg
Copy link
Author

Badg commented Apr 19, 2024

Thanks for the lightning-fast response! I updated to this, which fixed the issue: 🎉

[tool.ruff.lint.isort]
force-single-line = true
default-section = "third-party"
section-order = [
    "future",
    "standard-library",
    "third-party",
    "first-party",
    "local-folder",
    "testdeps",
]

[tool.ruff.lint.isort.sections]
testdeps = [
    "tt_website_testutils",
    "tt_website_tevecs",
    "tevec",
    "taev_ams_tevecs",]

I think I would still consider this a documentation issue, though maybe just with the release notes. I know there's explicitly no guarantee of stability right now, but especially when it comes to plugins like LSP packages for IDEs (where I don't realistically have the ability to pin a ruff version), it blindsided me. I'd say part of the difficulty is definitely the slightly-different spelling from isort's docs; when I'm unsure of something, I'm frequently bouncing back and forth between them, and it's just... hard to keep track of stuff, I guess.

Iunno. It's 23:00 on a Friday night here, and I've had a few beers, so maybe I'm just tired haha

@Badg
Copy link
Author

Badg commented Apr 19, 2024

PS feel free to close as resolved -- I'll leave it up to the team if y'all consider this to really be something that needs better documentation or not! :)

@augustelalande
Copy link
Contributor

augustelalande commented Apr 19, 2024

FYI LSP-ruff is using ruff-lsp 0.0.52 (latest is 0.0.53), and ruff-lsp 0.0.53 is only on ruff 0.2.2

@charliermarsh
Copy link
Member

I'll add a note on this in the documentation. I think this change was intentionally bringing us more in-line with isort's behavior, and I thought we were warning before when custom sections were omitted from section-order, but I might be totally wrong! I'm sorry for the trouble but glad we could resolve quickly.

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

Successfully merging a pull request may close this issue.

3 participants