ruff-pre-commit icon indicating copy to clipboard operation
ruff-pre-commit copied to clipboard

Ruff configuration from pyproject.toml is ignored

Open beajifu opened this issue 1 year ago • 36 comments

Hi,

I'm trying to use the pre-commit hook for Ruff, but it ignores my settings from the pyproject.toml. I want Ruff to ignore the line-length but if I commit a file with lines that exceed this limit it is complaining about it. If I use a ruff.toml to configure Ruff it works as expected.

I use version 0.0.292.

My settings in .pre-commit-config.yaml:

# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
repos:
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v3.2.0
    hooks:
    -   id: end-of-file-fixer
    -   id: check-added-large-files
-   repo: https://github.com/charliermarsh/ruff-pre-commit
    rev: v0.0.292
    hooks:
    -   id: ruff

Ruff configuration part in my pyproject.toml (same is used in the ruff.toml)

[tool.ruff]
select = ["E"]

# Never enforce `E501` (line length violations).
ignore = ["E501"]

beajifu avatar Oct 13 '23 14:10 beajifu

Hmm, a bit of a tough one to debug. When you switch between pyproject.toml and ruff.toml, are the files located in the same directory?

charliermarsh avatar Oct 13 '23 19:10 charliermarsh

Yes they are in the same directory.

I tried it on another laptop with a small example project and this problem did not occur. I try to find a way to reproduce the problem and give you an update.

beajifu avatar Oct 14 '23 20:10 beajifu

I could reproduce the problem and find a solution. If I use a project with already added code and a pyproject.toml to the git repo and then

  • add pre-commit to the virtual environment,
  • create a .pre-commit-config.yaml,
  • install the git hooks with pre-commit install
  • add the ruff configuration (ignore line length) to the pyproject.toml
  • try to only commit the python file that contains too long lines -> ruff complains about too long lines

When I first commit the .pre-commit-config.yaml and the pyproject.toml and after that commit the file with the long line then ruff ignores it as expected. Is this behavior expected?

beajifu avatar Oct 15 '23 11:10 beajifu

same issue with v0.1.0 but committing the config doesn't change anything

  • found the issue : the configuration needs to be added in the toml and we need to select the rule to use even for line-length
repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.1.0
    hooks:
      - id: ruff
        args: [--select=C901, --select=E501]

by default line-length is not checked, seems strange no ?

klow68 avatar Oct 19 '23 08:10 klow68

@klow68 - We removed E501 from the default configuration in v0.1.0 (https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md#breaking-changes), so I think that's working as intended.

charliermarsh avatar Oct 19 '23 16:10 charliermarsh

@beajifu I'm unable to reproduce with the steps mentioned although I'm using F401 as an example rule.

Gist: https://gist.github.com/dhruvmanila/8c796087330cb22699f3b20e929fde45

.
├── .pre-commit-config.yaml
├── pyproject.toml
└── test.py

Steps

  1. Install pre-commit and hooks
  2. Comment out the ignore in config file
$ pre-commit run
ruff.....................................................................Failed
- hook id: ruff
- exit code: 1

test.py:1:8: F401 [*] `os` imported but unused
Found 1 error.
[*] 1 potentially fixable with the --fix option.
  1. Uncomment the ignore in config file
$ pre-commit run
ruff.....................................................................Passed

dhruvmanila avatar Oct 30 '23 02:10 dhruvmanila

My configuration in pyproject.toml also seems to be having an issue.

Running ruff without any config: 3 errors

Running ruff with 3 select = ["ALL"] in pyproject.toml: 389 errors

Running ruff through pre-commit: 89 errors.

It seems like running ruff through pre-commit is using more rules than default but ignoring the rules in pyproject.toml

rpop0 avatar Nov 21 '23 15:11 rpop0

Chiming in just in case it may help someone. In my case, clearing the precommit cache ($HOME/.cache/pre-commit/) fixed a similarly sounding issue some time ago. Alas, I didn't keep the cache so I couldn't debug it further.

rytilahti avatar Nov 21 '23 16:11 rytilahti

This still seems to be an issue; using v0.1.7

caerulescens avatar Jan 12 '24 03:01 caerulescens

Would really appreciate if you could share some more information -- we weren't able to reproduce it above and it doesn't seem to be a common issue, so we need some help. What behavior are you seeing? What commands are you running? How is your project structured? Have you tried clearing the cache, as above? Thank you in advance!

charliermarsh avatar Jan 12 '24 04:01 charliermarsh

It seems I have more than one project reliably producing this bug (even after clearing caches), and I constructed a new project as a minimal example for @charliermarsh to reproduce the bug reliably. The new project I made ended up reading the pyproject.toml correctly, and I checked by causing N999: Invalid module name and then ignoring the rule: ignore = ["N999"] on a separate run. The bugged and new project that I compared both use ruff-pre-commit v0.1.7 and were invoked the same way. I don't think it's related to the version, but if I spend some more time I can isolate the issue.

caerulescens avatar Jan 12 '24 12:01 caerulescens

[tool.ruff]
select = ["E"]

# Never enforce `E501` (line length violations).
ignore = ["E501"]

don't these go in tool.ruff.lint?

rafalkrupinski avatar Jan 26 '24 23:01 rafalkrupinski

@rafalkrupinski I double checked docs, and you're right. I have been busy with other tasks, but I saved this thread for later. I'll retest, and repost.

caerulescens avatar Jan 27 '24 00:01 caerulescens

@caerulescens - Both are accepted right now -- putting ignore under [tool.ruff] and [tool.ruff.lint] are interchangeable. (We're moving towards the latter, but they both work as-is.)

charliermarsh avatar Jan 27 '24 00:01 charliermarsh

I realized the same thing just now; I'll retest

caerulescens avatar Jan 27 '24 00:01 caerulescens

I re-verified that I can [still] produce this bug with projects I'm working on; the [tool.ruff] section is being used. To isolate this bug, I'll probably copy/paste the repository locally, and I'll just start removing things until I can't reproduce the bug anymore, and then I'll post the results here. Earlier I tried to construct the minimum example from scratch, and I wasn't successful.

caerulescens avatar Jan 27 '24 01:01 caerulescens

@charliermarsh just created repo in which pre-commit hook ignores pyproject.toml config unlike the ruff itself. Hope this would help. https://github.com/Quatters/ruff-pre-commit-bug-demo

Quatters avatar Jan 30 '24 10:01 Quatters

@Quatters Hi, thanks for providing the repository for the problem you're facing. But, I don't think it's a bug with Ruff or pre-commit, I've described in detail about what's happening here: https://github.com/astral-sh/ruff/issues/9696#issuecomment-1916480913.

One solution would be to use pass_filenames: false in the pre-commit config so that the file discovery is done by Ruff taking into account the includes and excludes configured by the user in their pyproject.toml.

repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.1.15
    hooks:
      - id: ruff
        pass_filenames: false

This solves your problem.

That said, maybe Ruff's pre-commit hook should do that by default?

dhruvmanila avatar Jan 30 '24 10:01 dhruvmanila

@dhruvmanila thank you for the quick response. Yes, providing pass_filenames: false helped in my case.

That said, maybe Ruff's pre-commit hook should do that by default?

I'm not sure about doing that by default (will there be any side effects/breaking changes?). Anyway, I think information about pass_filenames would be useful in README.

Quatters avatar Jan 31 '24 01:01 Quatters

One solution would be to use pass_filenames: false in the pre-commit config so that the file discovery is done by Ruff taking into account the includes and excludes configured by the user in their pyproject.toml. That said, maybe Ruff's pre-commit hook should do that by default?

Wouldn't that make ruff to check all files every time? I think pre-commit users expect it to only check changed files, even if it is fast.

Even if pre-commit passes filenames to ruff check, I still don't see why it wouldn't reed the configuration...

rafalkrupinski avatar Jan 31 '24 20:01 rafalkrupinski

Agreed, running with pass_filenames: False is not a long term solution. Pre-commit needs to be the decider on what files get passed. This is one way of slowly fixing a large project - only touched files get linted/formatted.

It does help narrow down the issue though, good find!

dougthor42 avatar Jan 31 '24 22:01 dougthor42

@dhruvmanila That is not a usable work around for the reasons that @dougthor42 mentions.


@charliermarsh I played around with this issue tonight, and I have discovered more behavior surrounding this issue. I did something similar to the squeeze theorem but with code; I constructed a minimal project that doesn't have the issue, and I have a fully featured project with the issue (for the same rule). I've been removing parts of the larger project until I identify the issue.

pyproject.toml observations:

  1. If [tool.ruff.lint] is present, then all settings in [tool.ruff] are overridden; I'd imagine this is expected as [tool.ruff.lint] is set to overtake [tool.ruff]. Meaning, ignore = [] in [tool.ruff.lint] overrides all specified rules for ignore in [tool.ruff].
  2. I have no issue with ignoring D rules in [tool.ruff] or [tool.ruff.lint], but N999 is getting ignored regardless. Meaning, I have code that will invoke an error for D100, D104, and N999.
[tool.ruff]
line-length = 88  # Not relevant
target-version = "py310"  # Not relevant
src = ["src", "tests", "docs"]  # Not relevant
select = ["F", "E", "W", "C90", "S", "I", "N", "D", "UP"]
ignore = ["D100", "D104", "N999"]

[tool.ruff.pydocstyle]
convention = "pep257"  # Not relevant

Will produce:

{{ cookiecutter.repository_name }}/src/{{ cookiecutter.package_name }}/__init__.py:1:1: N999 Invalid module name: '{{ cookiecutter.package_name }}'
Found 1 error.

Then, removing rule D100 from ignore in [tool.ruff] or [tool.ruff.lint] will produce:

docs/conf.py:1:1: D100 Missing docstring in public module
tests/conftest.py:1:1: D100 Missing docstring in public module
tests/test_cookiecutter_terragrunt.py:1:1: D100 Missing docstring in public module
{{ cookiecutter.repository_name }}/src/{{ cookiecutter.package_name }}/__init__.py:1:1: N999 Invalid module name: '{{ cookiecutter.package_name }}'
Found 4 errors.

This means that [tool.ruff] and [tool.ruff.lint] within pyproject.toml is getting read by pre-commit; ~~this seems to be a specific issue that can be replicated with N999 but not maybe not other rules.~~ ~~This also means that [tool.ruff] and [tool.ruff.lint] suffer from the same problem, so the code that's causing the issue is shared by [tool.ruff] and [tool.ruff.lint], which probably doesn't narrow down much.~~ 3. I have a minimal project that can correctly ignore N999, yet it has the same pyproject.toml settings seen in (2).

caerulescens avatar Feb 05 '24 03:02 caerulescens

@charliermarsh I think I solved it! The problem occurs when there's another pyproject.toml within the project. The problem within the larger project was solved when I removed the second pyproject.toml within {{ cookiecutter.repository_name }}, which is used by cookiecutter. It seems that ruff prioritized [tool.ruff] in {{ cookiecutter.repository_name }}/pyproject.toml over the top-level pyproject.toml. This explains why this issue could be difficult to reproduce; it requires having multiple configurations for ruff present. I had the same D rules ignored in {{ cookiecutter.repository_name }}/pyproject.toml as the top-level pyproject.toml, which was misleading me during the debugging process.

This issue can be closed as ruff does read the pyproject.toml correctly. Maybe it would be helpful to note this behavior in the documentation if it's not already there? I think it could be useful to raise a warning when multiple configurations are found, and indicate that it could lead to unexpected behavior.

caerulescens avatar Feb 05 '24 03:02 caerulescens

@charliermarsh I think I solved it! The problem occurs when there's another pyproject.toml within the project. The problem within the larger project was solved when I removed the second pyproject.toml within {{ cookiecutter.repository_name }}, which is used by cookiecutter. It seems that ruff prioritized [tool.ruff] in {{ cookiecutter.repository_name }}/pyproject.toml over the top-level pyproject.toml. This explains why this issue could be difficult to reproduce; it requires having multiple configurations for ruff present. I had the same D rules ignored in {{ cookiecutter.repository_name }}/pyproject.toml as the top-level pyproject.toml, which was misleading me during the debugging process.

This issue can be closed as ruff does read the pyproject.toml correctly. Maybe it would be helpful to note this behavior in the documentation if it's not already there? I think it could be useful to raise a warning when multiple configurations are found, and indicate that it could lead to unexpected behavior.

This happens to me and I have a single pyproject.toml file

rpop0 avatar Feb 05 '24 08:02 rpop0

@rpop0 Could you elaborate further? This issue doesn't seem to be a problem; do you have other types of configurations present like ruff.toml? I identified my situation as user error, and I confirmed the precedence that ruff uses for reading settings from the pyproject.toml.

caerulescens avatar Feb 05 '24 11:02 caerulescens

@rpop0 Could you elaborate further? This issue doesn't seem to be a problem; do you have other types of configurations present like ruff.toml? I identified my situation as user error, and I confirmed the precedence that ruff uses for reading settings from the pyproject.toml.

No other types of configurations besides the single pyproject.toml file.

rpop0 avatar Feb 05 '24 11:02 rpop0

@rpop0 How are you reproducing this bug? Could you post a minimal example repository for us or maybe zip a folder and attach here?

caerulescens avatar Feb 05 '24 11:02 caerulescens

At the very least, the title for this issue is incorrect because [tool.ruff] and [tool.ruff.lint] within the pyproject.toml is getting read by pre-commit.

caerulescens avatar Feb 05 '24 11:02 caerulescens

I have been having the same issue where the ruff pre-commit hook was ignoring the line-length setting. I only had a single pyproject.toml file and I had the same behavior with a single ruff.toml file. The pass_filenames: false solution would allow the formatter to work properly, but then it would run on all files instead of only running on the newly committed changes.

I am using VSCode and the Ruff VSCode extension. Console commands in VSCode were working fine, but pre-commit would undo the formatting done by the VSCode extension.

When I disabled the VSCode extension, the problematic behavior went away and the precommit behavior starting working as expected. I was also able to reenable the Ruff VSCode extension and the issue has not yet returned. I hope this is helpful.

wiseyoungbuck avatar Mar 02 '24 18:03 wiseyoungbuck

I have the same problem that ruff-pre-commit does not load ruff config from pyproject.toml.

I resolved it by adding the config in the args.

  - repo: https://github.com/astral-sh/ruff-pre-commit
    # Ruff version.
    rev: v0.4.3
    hooks:
      # Run the linter.
      - id: ruff
        args: ["--select", "E,W,F,I,C,B,UP", "--ignore", "E203,B008,C901"]
      # Run the formatter.
      - id: ruff-format

ezeparziale avatar May 04 '24 21:05 ezeparziale