astral-sh / ruff-pre-commit

A pre-commit hook for Ruff.
Apache License 2.0
919 stars 41 forks source link

import sorting? #64

Open ringohoffman opened 8 months ago

ringohoffman commented 8 months ago

When I use this pyproject.toml config:

[tool.ruff]
select = [
    "I",  # isort formatting.
]

And this .pre-commit-config.yaml:

repos:
-   repo: https://github.com/charliermarsh/ruff-pre-commit
    rev: v0.1.13
    hooks:
    -   id: ruff
        types_or: [ python, pyi, jupyter ]
        args: [ --fix ]
    -   id: ruff-format
        types_or: [ python, pyi, jupyter ]

On code like this:

from typing import TypeVar, Generic

T = TypeVar("T")

class CameraOptimizerConfig(Generic[T]):
    pass

The ruff hook passes even though the imports aren't sorted:

$ pre-commit run --verbose
ruff.....................................................................Passed
- hook id: ruff
- duration: 0.07s
ruff-format..............................................................Failed
- hook id: ruff-format
- duration: 0.01s
- files were modified by this hook

But if I run ruff check --fix manually it does fix the imports:

$ ruff check nerfstudio/a.py --fix
Found 1 error (1 fixed, 0 remaining).

Am I missing some other configuration to get import sorting via the ruff pre-commit hook using args: [ --fix ]?

woctezuma commented 8 months ago

I have noticed this behavior. If you try to commit the file, I believe ruff would be applied.

I imagine this is due to the: 📦 Built-in caching, to avoid re-analyzing unchanged files

charliermarsh commented 8 months ago

This feels like https://github.com/astral-sh/ruff-pre-commit/issues/54 perhaps? Some people have said that clearing the pre-commit cache helps with this: https://github.com/astral-sh/ruff-pre-commit/issues/54#issuecomment-1821246684

ringohoffman commented 8 months ago

Some combination of these did fix it, though I'm having trouble reproducing the state I was in to confirm exactly what fixed it:

.PHONY: pre-commit/remove-cache
pre-commit/remove-cache: ## Remove all of pre-commit's cache
    rm -rf ~/.cache/pre-commit/

.PHONY: pre-commit/gc
pre-commit/gc: ## Clean unused cached repos.
    @pre-commit gc

.PHONY: pre-commit/autoupdate
pre-commit/autoupdate: ## Update pre-commit hook versions
    @pre-commit autoupdate

.PHONY: pre-commit/uninstall
pre-commit/uninstall: ## Uninstall pre-commit
    @pre-commit uninstall

.PHONY: pre-commit/install
pre-commit/install: ## Install pre-commit
    @pre-commit install --hook-type commit-msg --hook-type pre-push --hook-type pre-commit
taras0024 commented 8 months ago

@ringohoffman I have the same problem

HarryAnkers commented 6 months ago

Same problem

Tintean-Devops commented 6 months ago

I had this problem until I added --select I as an arg to the linter:

-   repo: https://github.com/astral-sh/ruff-pre-commit
    # Ruff version.
    rev: v0.3.5
    hooks:

        # Run the linter.
    -   id: ruff
        description: "Run 'ruff' for extremely fast Python linting"
        entry: ruff check --force-exclude
        language: python
        types_or: [ python, pyi, jupyter ]
        args: [ --fix, --select, I]
        require_serial: true
        minimum_pre_commit_version: "2.9.2"

I'm not using a pyproject.toml. If you are, this SO post may also help (not sure, but use extend-select = ["I"] rather than just select = ["I"]?): https://stackoverflow.com/questions/77876253/sort-imports-alphabetically-with-ruff

wu-clan commented 5 months ago

Same problem, the two solutions listed in the problem were tried, but the problem was not solved.

gboeer commented 5 months ago

I want to add my two cents to this. As I have described in https://github.com/astral-sh/ruff/issues/8926#issuecomment-2049816399 I was having a similar issue where Ruff supposedly wasn't used correctly in the pre-commit-hook.

Turns out, I just didn't commit my pyproject.toml after adding the new Ruff settings (e.g. to perform import sorting). After committing the pyproject.toml the subsequent linting/sorting errors get caught and reformatted as expected.

aaraney commented 5 months ago

You can list pre-commit hook id's multiple times and pre-commit will not deduplicate them (notice ruff listed twice). So, to lint -> sort imports -> format, you can do the following:

repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.4.1
    hooks:
      - id: ruff
        name: lint with ruff
      - id: ruff
        name: sort imports with ruff
        args: [--select, I, --fix]
      - id: ruff-format
        name: format with ruff

Unlike when using black and isort with pre-commit where you need to use isort then black, I don't think the order matters. So, if you sort imports then format or format then sort imports shouldn't matter? There could be a ruff configuration permutation such that this is not true, but with the default settings, the order does not seem to matter.