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

import sorting? #64

Open
ringohoffman opened this issue Jan 14, 2024 · 9 comments
Open

import sorting? #64

ringohoffman opened this issue Jan 14, 2024 · 9 comments
Labels
question Further information is requested

Comments

@ringohoffman
Copy link

ringohoffman commented Jan 14, 2024

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
Copy link

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
Copy link
Member

This feels like #54 perhaps? Some people have said that clearing the pre-commit cache helps with this: #54 (comment)

@charliermarsh charliermarsh added the question Further information is requested label Jan 14, 2024
@ringohoffman
Copy link
Author

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
Copy link

@ringohoffman I have the same problem

@HarryAnkers
Copy link

Same problem

@Tintean-Devops
Copy link

Tintean-Devops commented Apr 8, 2024

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
Copy link

wu-clan commented Apr 10, 2024

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

@gboeer
Copy link

gboeer commented Apr 12, 2024

I want to add my two cents to this. As I have described in astral-sh/ruff#8926 (comment) 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
Copy link

aaraney commented Apr 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

9 participants