Rename venv-path to python
#16347
Rename venv-path to python
#16347
Conversation
MichaReiser
requested review from
carljm,
AlexWaygood and
sharkdp
as code owners
| } | ||
| }) | ||
| .transpose()? | ||
| .unwrap_or_else(|| cli_base_path.clone()); | ||
| .unwrap_or_else(|| cwd.clone()); |
There was a problem hiding this comment.
I noticed while testing that relative path arguments on the CLI were incorrectly anchored to the project directory instead of the cwd. For example, the python path for --project ../test --python ../test/venv resolved to ../test/test/venv.
| @@ -69,7 +69,7 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> { | |||
| let _guard = setup_tracing(verbosity)?; | |||
|
|
|||
| // The base path to which all CLI arguments are relative to. | |||
There was a problem hiding this comment.
This comment seems out of date (or at least badly located) now?
There was a problem hiding this comment.
This is still true. We relativize the paths in RelativePathBuf::absolute. The main change here is that we don't use the project path as cwd when it is set (which is incorrect)
This is from the PR summary, not the PR itself, but in case it represents an actual misunderstanding: |
You give me too much credit. I was just still confused by how the directory structure looks. |
* main: (38 commits) [red-knot] Use arena-allocated association lists for narrowing constraints (#16306) [red-knot] Rewrite `Type::try_iterate()` to improve type inference and diagnostic messages (#16321) Add issue templates (#16213) Normalize inconsistent markdown headings in docstrings (#16364) [red-knot] Better diagnostics for method calls (#16362) [red-knot] Add argfile and windows glob path support (#16353) [red-knot] Handle pipe-errors gracefully (#16354) Rename `venv-path` to `python` (#16347) [red-knot] Fixup some formatting in `infer.rs` (#16348) [red-knot] Restrict visibility of more things in `class.rs` (#16346) [red-knot] Add diagnostic for class-object access to pure instance variables (#16036) Add `per-file-target-version` option (#16257) [PLW1507] Mark fix unsafe (#16343) [red-knot] Add a test to ensure that `KnownClass::try_from_file_and_name()` is kept up to date (#16326) Extract class and instance types (#16337) Re-order changelog entries for 0.9.7 (#16344) [red-knot] Add support for `@classmethod`s (#16305) Update Salsa (#16338) Update Salsa part 1 (#16340) Upgrade Rust toolchain to 1.85.0 (#16339) ...
Summary
Closes #15530
This PR renames the CLI argument
--venv-pathand theenvironment.venv-pathoption to--pythonandenvironment.pythonto align with uv's python option.
The Python option takes a path to a virtual environment or Python system installation (any directory that has a
lib/python3.x/site-packagessubdirectory on unix).This PR doesn't yet add support for:
uv supports both (and more). The CLI proposal does mention that we should support python executables. I recommend to add support for this, if wanted, as a separate PR
Test plan
I ran Red Knot over Black and both main and this PR produce the same diagnostic when pointing it to a venv path.