Allow users to extend the set of included files via `include` #3914

charliermarsh · 2023-04-08T04:14:44Z

Summary

This PR adds an include mechanism to Ruff's configuration, which mirrors exclude (and extend_exclude), but is applied after exclusions, and allows users to include files beyond *.pyi and *.py (the default) when running Ruff.

For example, if users want to lint .pyw files in addition to .pyi and py files, they could add:

[tool.ruff]
extend-include = ["*.pyw"]

Closes #3410.

Closes #3783.

Closes #2192 (by providing a workaround).

charliermarsh · 2023-04-08T04:18:02Z

This introduces a small performance regression (from ~247ms to ~251ms on CPython).

github-actions · 2023-04-08T04:25:24Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.02     15.3±0.71ms     2.7 MB/sec    1.00     15.0±0.79ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.04      3.9±0.21ms     4.3 MB/sec    1.00      3.7±0.21ms     4.4 MB/sec
linter/all-rules/numpy/globals.py          1.08   526.0±15.70µs     5.6 MB/sec    1.00   486.2±31.93µs     6.1 MB/sec
linter/all-rules/pydantic/types.py         1.04      6.7±0.38ms     3.8 MB/sec    1.00      6.5±0.42ms     3.9 MB/sec
linter/default-rules/large/dataset.py      1.01      8.0±0.36ms     5.1 MB/sec    1.00      7.9±0.38ms     5.2 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.03  1871.4±78.63µs     8.9 MB/sec    1.00  1812.1±80.79µs     9.2 MB/sec
linter/default-rules/numpy/globals.py      1.00   194.7±12.37µs    15.2 MB/sec    1.04   201.6±10.75µs    14.6 MB/sec
linter/default-rules/pydantic/types.py     1.06      3.8±0.18ms     6.7 MB/sec    1.00      3.6±0.23ms     7.1 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     16.3±0.36ms     2.5 MB/sec    1.00     16.2±0.16ms     2.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.1±0.05ms     4.0 MB/sec    1.01      4.2±0.05ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.00    491.3±6.23µs     6.0 MB/sec    1.02    498.7±8.13µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.9±0.15ms     3.7 MB/sec    1.00      6.9±0.10ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.00      8.1±0.07ms     5.0 MB/sec    1.03      8.4±0.11ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1777.4±30.43µs     9.4 MB/sec    1.04  1846.4±30.48µs     9.0 MB/sec
linter/default-rules/numpy/globals.py      1.00    192.7±4.41µs    15.3 MB/sec    1.02    197.5±5.24µs    14.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.7±0.05ms     6.8 MB/sec    1.02      3.8±0.08ms     6.7 MB/sec

charliermarsh · 2023-04-08T15:15:25Z

I think we can improve performance by tracking whether any settings have custom inclusions or exclusions, and avoiding the settings resolution altogether if not. But, that would be better as a separate PR anyway.

MichaReiser · 2023-04-11T06:24:51Z

I think this is a good change to unlock users, even though I would have preferred a new settings schema first, but this is taking too long.

This PR adds an include mechanism to Ruff's configuration, which mirrors exclude (and extend_exclude), but is applied after exclusions, and allows users to include files beyond *.pyi and *.py (the default) when running Ruff.

I would have expected the opposite: includes define the initial file set and exclude filter the files. What's your motivation for choosing this precedence?

Do you know how flake8 supports these use cases? I wasn't able to find an include option.

MichaReiser · 2023-04-11T06:21:34Z

crates/ruff/src/resolver.rs

+                    if settings.include.is_match(path) {
+                        debug!("Included path via `include`: {:?}", path);
+                        true
+                    } else if settings.extend_include.is_match(path) {


Nit: I wonder if it would improve performance if we merge extend_include and include.

It's nice to have the separate debug "reason" but I agree, in theory at least it would be faster. In practice (and in my benchmarks), extend_include is almost always empty.

charliermarsh · 2023-04-12T03:30:41Z

crates/ruff/src/resolver.rs

+                    // Otherwise, check if the file is included.
+                    let path = entry.path();
+                    let resolver = resolver.read().unwrap();
+                    let settings = resolver.resolve(path, pyproject_strategy);


I think the real performance penalty is having to resolve settings for every file.

charliermarsh · 2023-04-12T03:32:14Z

I would have expected the opposite: includes define the initial file set and exclude filter the files. What's your motivation for choosing this precedence?

Do you know how flake8 supports these use cases? I wasn't able to find an include option.

Good question -- the precedence here is from Black, here's the documentation:

  --include TEXT                  A regular expression that matches files and
                                  directories that should be included on
                                  recursive searches. An empty value means all
                                  files are included regardless of the name.
                                  Use forward slashes for directories on all
                                  platforms (Windows, too). Exclusions are
                                  calculated first, inclusions later.
                                  [default: \.pyi?$]
  --exclude TEXT                  A regular expression that matches files and
                                  directories that should be excluded on
                                  recursive searches. An empty value means no
                                  paths are excluded. Use forward slashes for
                                  directories on all platforms (Windows, too).
                                  Exclusions are calculated first, inclusions
                                  later. [default: /(\.direnv|\.eggs|\.git|\.h
                                  g|\.mypy_cache|\.nox|\.tox|\.venv|venv|\.svn
                                  |\.ipynb_checkpoints|_build|buck-

charliermarsh · 2023-04-12T03:33:49Z

(Related to the above docs: I don't really know what it would mean to apply an inclusion to a directory in this context, since we traverse over all directories by default and exclusions are applied first (so you can never include a subdirectory within an excluded parent). So I'm only testing inclusions against files.)

charliermarsh · 2023-04-12T03:38:34Z

Relevant stuff from Black:

psf/black#281
psf/black#270

charliermarsh · 2023-04-12T03:39:27Z

AFAICT, they also only apply inclusion checks to individual files.

## Summary Remove the `jupyter_notebook` feature. This is now handled by the `include` feature introduced in #3914 ## Test Plan I have confirmed that running `cargo run --bin ruff -- check --select F841 --no-cache dir_with_notebook/` doesn't pick them up, but adding `include = ["*.ipynb"]` in `pyproject.toml` makes the notebooks found.

Add include

efaebd7

charliermarsh requested a review from MichaReiser April 8, 2023 15:14

MichaReiser approved these changes Apr 11, 2023

View reviewed changes

charliermarsh commented Apr 12, 2023

View reviewed changes

charliermarsh merged commit b999e4b into main Apr 12, 2023

charliermarsh deleted the charlie/include branch April 12, 2023 03:39

spaceone mentioned this pull request Apr 12, 2023

operate on files with certain hashbang #2192

Closed

p-linnane mentioned this pull request Apr 20, 2023

ruff 0.0.262 Homebrew/homebrew-core#128839

Merged

charliermarsh mentioned this pull request Aug 23, 2023

Add a include parameter to allow expanding scope of included files #3410

Closed

charliermarsh mentioned this pull request Sep 4, 2024

Include .pyw files by default #13246

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow users to extend the set of included files via `include` #3914

Allow users to extend the set of included files via `include` #3914

charliermarsh commented Apr 8, 2023 •

edited

Loading

charliermarsh commented Apr 8, 2023

github-actions bot commented Apr 8, 2023 •

edited

Loading

charliermarsh commented Apr 8, 2023

MichaReiser commented Apr 11, 2023 •

edited

Loading

MichaReiser Apr 11, 2023

charliermarsh Apr 12, 2023

charliermarsh Apr 12, 2023

charliermarsh commented Apr 12, 2023

charliermarsh commented Apr 12, 2023

charliermarsh commented Apr 12, 2023

charliermarsh commented Apr 12, 2023

Allow users to extend the set of included files via include #3914

Allow users to extend the set of included files via include #3914

Conversation

charliermarsh commented Apr 8, 2023 • edited Loading

Summary

charliermarsh commented Apr 8, 2023

github-actions bot commented Apr 8, 2023 • edited Loading

PR Check Results

Ecosystem

Benchmark

Linux

Windows

charliermarsh commented Apr 8, 2023

MichaReiser commented Apr 11, 2023 • edited Loading

MichaReiser Apr 11, 2023

Choose a reason for hiding this comment

charliermarsh Apr 12, 2023

Choose a reason for hiding this comment

charliermarsh Apr 12, 2023

Choose a reason for hiding this comment

charliermarsh commented Apr 12, 2023

charliermarsh commented Apr 12, 2023

charliermarsh commented Apr 12, 2023

charliermarsh commented Apr 12, 2023

Allow users to extend the set of included files via `include` #3914

Allow users to extend the set of included files via `include` #3914

charliermarsh commented Apr 8, 2023 •

edited

Loading

github-actions bot commented Apr 8, 2023 •

edited

Loading

MichaReiser commented Apr 11, 2023 •

edited

Loading