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 clash with ruff I001 when module has digit #6043

Closed
mmarras opened this issue Jul 24, 2023 · 5 comments
Closed

isort clash with ruff I001 when module has digit #6043

mmarras opened this issue Jul 24, 2023 · 5 comments
Labels
isort Related to import sorting needs-decision Awaiting a decision from a maintainer

Comments

@mmarras
Copy link

mmarras commented Jul 24, 2023

isort sorts imports in a way that induces ruff to flag I001, aka ruff and isort want to sort the imports in different ways (I suspect when module has a number in name).

after running isort

from basic.views import datamore_login_required
from geln_main import config as config_main
from geln_olga2 import config
from geln_olga2.models import Data
from geln_olga.pytools import olga
from geln_olga.views import get_histogram_data, validate_parse_sp2files
from global_log_usage.views import create_hit

then

geln_olga2/views.py:1:1: I001 [*] Import block is un-sorted or un-formatted
Found 1 error.
[*] 1 potentially fixable with the --fix option.

after running ruff auto-correct (which I only apply manually to see the difference)

from basic.views import datamore_login_required
from geln_main import config as config_main
from geln_olga.pytools import olga
from geln_olga.views import get_histogram_data, validate_parse_sp2files
from geln_olga2 import config
from geln_olga2.models import Data
from global_log_usage.views import create_hit

Relevant settings are:

.pre-commit-config.yaml

- repo: https://github.com/pre-commit/mirrors-isort
    rev: v5.10.1
    hooks:
      - id: isort
        args: ["--profile", "black"]
- repo: https://github.com/charliermarsh/ruff-pre-commit
    rev: "v0.0.280"
    hooks:
      - id: ruff
@mmarras mmarras changed the title isort clash with ruff I001 isort clash with ruff I001 when module has digit Jul 24, 2023
@charliermarsh
Copy link
Member

Hah interesting

@charliermarsh charliermarsh added isort Related to import sorting needs-decision Awaiting a decision from a maintainer labels Jul 24, 2023
@charliermarsh
Copy link
Member

I'm trying to understand why isort would use that ordering. It's strange because (e.g.) geln_olgaZ (or any letter) would cause isort to use Ruff's import ordering. So it needs to be treating numerical suffixes as lexicographically lower.

@zanieb
Copy link
Member

zanieb commented Jul 24, 2023

@charliermarsh
Copy link
Member

Ahhh thank you, so helpful.

My gut reaction is that this looks like an unintended behavior in isort... They're trying to ensure that they respect this:

from geln_olga2.models import Data
from geln_olga10.models import Data

But that's leading them to put numbers before non-numerical suffixes. Ruff seems to do the right thing -- it respects this:

from geln_olga2.models import Data
from geln_olga10.models import Data

But also this:

from geln_olga.models import Data
from geln_olga2.models import Data

@charliermarsh
Copy link
Member

I think I'm gonna close with the understanding that we do deviate from upstream when we feel that our behavior represents a bug fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
isort Related to import sorting needs-decision Awaiting a decision from a maintainer
Projects
None yet
Development

No branches or pull requests

3 participants