Skip to content

Comments

Allow passing a virtual environment to ruff analyze graph#17743

Merged
ntBre merged 2 commits intomainfrom
brent/ruff-analyze-workspace
May 1, 2025
Merged

Allow passing a virtual environment to ruff analyze graph#17743
ntBre merged 2 commits intomainfrom
brent/ruff-analyze-workspace

Conversation

@ntBre
Copy link
Contributor

@ntBre ntBre commented Apr 30, 2025

Summary

Fixes #16598 by adding the --python flag to ruff analyze graph, which adds a PythonPath to the SearchPathSettings for module resolution. For the albatross-virtual-workspace example from the uv repo, this updates the output from the initial issue:

> ruff analyze graph packages/albatross
{
  "packages/albatross/check_installed_albatross.py": [
    "packages/albatross/src/albatross/__init__.py"
  ],
  "packages/albatross/src/albatross/__init__.py": []
}

To include both the the workspace bird_feeder import and the third-party tqdm import in the output:

> myruff analyze graph packages/albatross --python .venv
{
  "packages/albatross/check_installed_albatross.py": [
    "packages/albatross/src/albatross/__init__.py"
  ],
  "packages/albatross/src/albatross/__init__.py": [
    ".venv/lib/python3.12/site-packages/tqdm/__init__.py",
    "packages/bird-feeder/src/bird_feeder/__init__.py"
  ]
}

Note the hash in the uv link! I was temporarily very confused why my local tests were showing an iniconfig import instead of tqdm until I realized that the example has been updated on the uv main branch, which I had locally.

Test Plan

A new integration test with a stripped down venv based on the albatross example.

@ntBre
Allow passing a virtual environment to ruff analyze graph
6a7b16b 
Summary
--

Fixes #16598 by adding the `--python` flag to `ruff analyze graph`, which adds a
`PythonPath` to the `SearchPathSettings` for module resolution. For the
[albatross-virtual-workspace] example from the uv repo this updates the output
from the initial issue:

```shell
> ruff analyze graph packages/albatross
{
  "packages/albatross/check_installed_albatross.py": [
    "packages/albatross/src/albatross/__init__.py"
  ],
  "packages/albatross/src/albatross/__init__.py": []
}
```

To include both the the workspace `bird_feeder` import _and_ the third-party
`tqdm` import in the output:

```shell
> myruff analyze graph packages/albatross --python .venv
{
  "packages/albatross/check_installed_albatross.py": [
    "packages/albatross/src/albatross/__init__.py"
  ],
  "packages/albatross/src/albatross/__init__.py": [
    ".venv/lib/python3.12/site-packages/tqdm/__init__.py",
    "packages/bird-feeder/src/bird_feeder/__init__.py"
  ]
}
```

Note the hash in the uv link! I was temporarily very confused why my local tests
were showing an `iniconfig` import instead of `tqdm` until I realized that the
example has been updated on the uv main branch, which I had locally.

Test Plan
--

A new integration test with a stripped down venv based on the `albatross`
example.

[albatross-virtual-workspace]: https://github.com/astral-sh/uv/tree/aa629c4a54c31d6132ab1655b90dd7542c17d120/scripts/workspaces/albatross-virtual-workspace
@ntBre ntBre added the analyze Related to Ruff analyze functionality label Apr 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 30, 2025 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre marked this pull request as ready for review April 30, 2025 17:33
@ntBre ntBre requested a review from MichaReiser April 30, 2025 17:33
MichaReiser
MichaReiser approved these changes Apr 30, 2025
View reviewed changes
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

Can you test what happens if you provide an invalid path?

@ntBre
Copy link
Contributor Author

ntBre commented Apr 30, 2025

Ah good idea, I ran into a couple of these while trying to set up the working test. Is this what you expected?

----- stderr -----                                                                                                     
ruff failed                                                                                                            
  Cause: Invalid search path settings                                                                                  
  Cause: Failed to discover the site-packages directory: Invalid `--python` argument: `none` could not be canonicalized

I think that looks pretty reasonable. Something like venv dir `none` does not exist could possibly be nicer, but I think the canonicalized message is bubbling up from red-knot. I could try to check manually a bit earlier if we wanted, though.

It's also important that the flag name matches red-knot, which I'm guessing you were also thinking about here!

@MichaReiser
Copy link
Member

MichaReiser commented May 1, 2025 •
edited
Loading

I mainly wanted to make sure that ruff doesn't panic (I wasn't sure if there are any unwraps). I assume none was what you provided on the CLI. This looks good.

It's also important that the flag name matches red-knot, which I'm guessing you were also thinking about here!

You give me too much credit 😅

@ntBre ntBre merged commit 163d526 into main May 1, 2025
34 checks passed
@ntBre ntBre deleted the brent/ruff-analyze-workspace branch May 1, 2025 15:29
dcreager added a commit that referenced this pull request May 2, 2025
@dcreager
Merge branch 'main' into dcreager/legacy-class
1ce6bab 
* main:
  [red-knot] Refactor: no mutability in call APIs (#17788)
  [red-knot] Fix panic for `tuple[x[y]]` string annotation (#17787)
  [red-knot] Implicit instance attributes in generic methods (#17769)
  doc: Add link to `check-typed-exception` from `S110` and `S112` (#17786)
  Fix module name in ASYNC110, 115, and 116 fixes (#17774)
  [red-knot] More informative hover-types for assignments (#17762)
  [syntax-errors] Use consistent message for bad starred expression usage. (#17772)
  red_knot_server: add auto-completion MVP
  Allow passing a virtual environment to `ruff analyze graph` (#17743)
  Bump 0.11.8 (#17766)
  [`flake8-use-pathlib`] Fix `PTH104`false positive when `rename` is passed a file descriptor (#17712)
@BrewTestBot BrewTestBot mentioned this pull request May 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

analyze Related to Ruff analyze functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow ruff analyze to be run across a uv workspace packages

2 participants

@ntBre @MichaReiser