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

ruff format doesn't sort imports, leaves file unchanged #8926

Closed
alexei opened this issue Nov 30, 2023 · 12 comments
Closed

ruff format doesn't sort imports, leaves file unchanged #8926

alexei opened this issue Nov 30, 2023 · 12 comments
Labels
question Asking for support or clarification

Comments

@alexei
Copy link

alexei commented Nov 30, 2023

I'm looking into using ruff (0.1.6 on Python 3.12) for formatting and... it doesn't appear to be doing anything, at least in terms of replicating isort behaviour.

Input

from _common import foo
import dataclasses as dc
import psycopg
from typing import Any
import hashlib
from foo.bar import baz
from functools import partial
from time import monotonic
from enum import Enum, auto
from uuid import UUID

Command

$ python -m ruff format -v ./test_ruff.py
[2023-11-30][15:29:28][ruff_cli::resolve][DEBUG] Using Ruff default settings
[2023-11-30][15:29:28][ruff_cli::commands::format][DEBUG] format_path; path=.../test_ruff.py
[2023-11-30][15:29:28][tracing::span][DEBUG] Printer::print;
[2023-11-30][15:29:28][ruff_cli::commands::format][DEBUG] Formatted 1 files in 191.92µs
1 file left unchanged

Expected result

import dataclasses as dc
from enum import Enum, auto
from functools import partial
import hashlib
from time import monotonic
from typing import Any
from uuid import UUID

import psycopg

from _common import foo
from foo.bar import baz

Or something like that.

Actual result

The file is unchanged.


What am I missing?

@charliermarsh
Copy link
Member

In Ruff, import sorting and re-categorization is part of the linter, not the formatter. The formatter will re-format imports, but it won't rearrange or regroup them, because the formatter maintains the invariant that it doesn't modify the program's AST (i.e., its semantics and behavior).

To get isort-like behavior, you'd want to run ruff check --fix with --select I or adding extend-select = ["I"] to your pyproject.toml or ruff.toml.

@zanieb
Copy link
Member

zanieb commented Nov 30, 2023

See also #8087, #8612, and #8367

@zanieb zanieb closed this as completed Nov 30, 2023
@zanieb zanieb added the question Asking for support or clarification label Nov 30, 2023
@alexei
Copy link
Author

alexei commented Nov 30, 2023

Got it, thanks!

@Hubro
Copy link

Hubro commented Feb 27, 2024

While the rationale makes complete sense, this does make it pretty awkward and unintuitive to sort import statements as part of auto-formatting in editor configs and such. Would it be a possibility to add a shortcut for this as a flag to ruff format, just as a UX improvement?

--sort-imports
    Sort imports after formatting, equivalent to running `ruff check --fix --select I`

@alexei
Copy link
Author

alexei commented Feb 27, 2024

I agree it sounds technically correct, but a little awkward from a practical standpoint. When I need to format the code, I use a script that runs:

ruff check . --select I --fix
ruff format .

@ruslaniv
Copy link

ruslaniv commented Apr 8, 2024

To get isort-like behavior, you'd want to run ruff check --fix with --select I or adding extend-select = ["I"] to your pyproject.toml or ruff.toml.

I have added

[tool.ruff.lint]
extend-select = ["I"]

to pyproject toml, so now when running

ruff check --fix config.py

I get

config.py:37:1: E402 Module level import not at top of file
Found 1 error.

but nothing gets resorted in the file.
How do I tell ruff to actually physically resort the imports in the file just like iSort does

@alexei
Copy link
Author

alexei commented Apr 8, 2024

@ruslaniv have you tried select instead of extend-select?

@ruslaniv
Copy link

ruslaniv commented Apr 8, 2024

Yes, just tried it

[tool.ruff.lint]
select = ["I"]

and in this case it's not even picking up the error:

ruff check --fix config.py           
All checks passed!

@MichaReiser
Copy link
Member

@ruslaniv please consider creating a new issue, ideally with minimal repro.

Can you try running ruff check config.py --show-settings to see what rules are selected?

@gboeer
Copy link

gboeer commented Apr 11, 2024

EDIT: In my case, I just forgot to commit the pyproject.toml after having added the ruff settings (e.g. to do import sorting). After committing the pyproject.toml the subsequent linting/sorting errors get caught and reformatted as expected in my pre-commit.

I can add a similar observation to this. Having a simple file like:

from pathlib import Path
import os

print(Path(os.getcwd()))

and a pyproject.toml entry:

[tool.ruff.lint]
select = ["I"]

The cli command behaves as expected:

ruff check temp.py
temp.py:1:1: I001 [*] Import block is un-sorted or un-formatted
Found 1 error.
[*] 1 fixable with the `--fix` option.

And will correctly fix the import statement when I add --fix

However, in my pre-commit hook the file will not be fixed but accepted as is (with the wrongly ordered inputs).

My pre-commit is configured as:

- repo: https://github.com/astral-sh/ruff-pre-commit
  # Ruff version.
  rev: v0.3.5
  hooks:
    # Run the linter.
    - id: ruff
      args: [ --fix ]
    # Run the formatter.
    - id: ruff-format

@charliermarsh
Copy link
Member

@gboeer - This sounds similar to astral-sh/ruff-pre-commit#64 -- perhaps chime in there?

@zhou13
Copy link

zhou13 commented Jun 19, 2024

This issue causes some inconvenience using the sorting functionality:

  • It is intuitive to think ruff format will help you reorder (i.e., format) the import, rather than using ruff check --fix;
  • While it might be not true, but I would imagine that ruff check --fix might be somehow dangerous and thus hesitant to run it too often. However, sorting the import should be saf enough to run on every file save in IDE.
  • This makes isort function from ruff lsp-server awkward to use in many IDEs:
    • Normally, IDE has an option like format_on_save using LSP, so it is very easy to run ruff format in your IDE;
    • To sort the import, one needs to run a code action. This takes more keystrokes, and it is much harder to configure to run it with format_on_save in general.

I do understand that currently import sorting and re-categorization is implemented as the linter, not the formatter, which might make sense in terms of the function group. But as a user, I think the software UI ergonomics is more important here.

boriel added a commit to boriel-basic/zxbasic that referenced this issue Jun 24, 2024
Currently ruff format does not sort imports
(see astral-sh/ruff#8926). This means
that executing poe format was not sorting them which can cause
errors during linting check in the pipeline.
perlinm added a commit to Infleqtion/client-superstaq that referenced this issue Aug 8, 2024
[Ruff](https://docs.astral.sh/ruff/)
([GitHub](https://github.com/astral-sh/ruff)) is a Python linter and
formatter that plays the role of `black`, `isort`, `flake8`, and
`pylint`, and does it all much faster (for whatever that is worth). This
PR adds the **option** to use `ruff` in place of these tools to
`checks-superstaq`.

Specifically, this PR functionally adds:
- `ruff_format_.py` (which aspires to replace one day replace
`format_.py`)
- `ruff_lint_.py` (which aspires to one day replace `flake8_.py` and
`pylint_.py`, at which point it can be renamed to (say) `lint_.py`).
- A `--ruff` flag to `checks_superstaq.all_` to use `ruff` instead of
the previous formatters+linters.

Note that `ruff_format_.py` has a `--fix` option instead of `--apply`
for parity with the `--fix` option recognized by `ruff check` (which is
called by `ruff_lint_.py`). That's because `ruff format` [refuses to
change a program's
AST](astral-sh/ruff#8926 (comment)),
so it's not allowed to change import order. Import order therefore has
to be handled by `ruff check` with `./checks/ruff_lint_.py --fix`.
(Incidentally, maybe `ruff_lint_.py` should be renamed to `ruff_check_`
because it calls `ruff check`? But then if+when this repo switches to
using ruff it will feel silly to call `checks/check_.py`. Or maybe
if+when `ruff` takes over we can have `ruff_format.py` and
`ruff_check.py`?)

`ruff` comes with many reasonable defaults, so I did not need to add
much to `pyproject.toml`. Having said that, `checks-superstaq` has
extensive configuration of `flake8` and `pylint`, and I am sure that not
all of these linting rules are enforced by ruff. I leave it up to you
all @Infleqtion to make the call on whether it's okay to merge in a
"partial" switch to ruff (at the cost of having duplicate
formatting/linting tools in the repo) vs. whether you'd like to do one
big switch (configs and all).

At the moment, all checks of `ruff_lint_.py` pass, and `ruff_format_.py`
asks for a few minor formatting changes. The requested formatting
changes are not included in this PR (though if I do make the changes,
`checks/format_.py` is happy with all except those made to one `.pynb`
file).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Asking for support or clarification
Projects
None yet
Development

No branches or pull requests

8 participants