`python pin` now finds workspace root folder when called in subfolder by silvanocerza · Pull Request #5035 · astral-sh/uv

silvanocerza · 2024-07-13T09:23:07Z

Summary

This PR changes python pin behaviour when called in a workspace subfolder. Previously it would only read or create a .python-version file in the current working directory, now instead it will first search for the workspace root folder and read or create the .python-version file in that folder.

We assume the root folder contains a valid pyproject.toml file.

If we're not in a workspace use the previous behaviour and creates or reads .python-version in the current working directory.

Test Plan

I added a new test to verify this new behaviour, and run all tests locally.

zanieb · 2024-07-13T15:38:31Z

We might want to make this asymmetric per #4971 (comment) and read from the workspace root by default but, when writing, require an opt-in via --workspace otherwise we write to the current directory. Or should we require an opt-out via --isolated to ignore the workspace and write to the working directory? What do you think? I'm not 100% certain what the best UX is yet.

T-256 · 2024-07-13T15:54:14Z

Or should we use --isolated to ignore the workspace and write to the working directory?

IMO this is more correct to me, since pinned python version is usually used for venv creation/recreation and venv is in workspace root.

silvanocerza · 2024-07-13T17:36:04Z

I would agree with @T-256 here, it makes sense to me using the --isolated option to ignore the workspace.
Should I got with that?

silvanocerza · 2024-07-14T15:08:45Z

Rebased to bring in CI fix from #5034 and remove some commented out code that I missed.

crates/uv/tests/python_pin.rs

crates/uv/src/commands/python/pin.rs

crates/uv-python/src/version_files.rs

zanieb · 2024-07-14T16:22:51Z

Sure let's just make --isolated ignore the workspace to start and we can tackle more complex behavior separately?

silvanocerza · 2024-07-15T15:04:06Z

Ready for review again. 👍

silvanocerza · 2024-07-15T15:06:51Z

crates/uv/src/commands/python/pin.rs

-    if exists {
-        writeln!(printer.stdout(), "Replaced existing pin with `{output}`")?;
+    let mut message = if exists {
+        format!("Replaced existing pin with `{output}`")
    } else {
-        writeln!(printer.stdout(), "Pinned to `{output}`")?;
-    }
+        format!("Pinned to `{output}`")
+    };
+
+    if target_dir != std::env::current_dir()? {
+        // Print the version file use to pin only
+        // if it's not in the current working directory
+        message = format!("{message} in `{}`", version_file.display());
+    };
+
+    writeln!(printer.stdout(), "{message}")?;


I'm not sure about this part, would something like this be better? 🤔

let message = if exists { format!("Replaced existing pin with `{output}`") } else { format!("Pinned to `{output}`") }; let message = if target_dir != std::env::current_dir()? { // Print the version file use to pin only // if it's not in the current working directory format!("{message} in `{}`", version_file.display()) } else { message }; writeln!(printer.stdout(), "{message}")?;

zanieb · 2024-07-19T18:12:01Z

Going to be lots of conflicts with #5215 and #4989 — I'll own resolving them.

zanieb · 2024-07-19T19:14:59Z

Okay I resolved the conflicts but then realized we need to make a lot of other changes here, e.g. every other command needs to respect pins in the workspace. Kind of complicated, I need to think about it before moving forward.

zanieb · 2024-07-19T19:44:59Z

Here's where I left off https://github.com/astral-sh/uv/compare/zb/pin-find

silvanocerza · 2024-07-20T08:45:06Z

If you can give me a list of the commands that actually needs to respect the pin I should be able to handle that, I'm just unsure about all the commands.

Also probably we need to isolate the logic that finds the pin location, it would simplify implementing it for the other commands.

zanieb · 2024-07-20T16:10:28Z

I think we need to walk up the directory tree looking for a version file like we do with pyproject.toml files for project roots. Then, every command that needs a Python interpreter needs to respect the version file when determining the default. We don't need to do it all at once, I think outside the preview commands only venv supports version files right now and it's gated by preview.

We may or may not want to stop searching at a boundary where we find a pyproject.toml. If we don't, then we need to check if the version is pinned to something incompatible with the current project's Python requirement. If the pin comes from a directory above the project, then we probably need to warn and use the Requires-Python value instead.

It seems simplest to do something like:

Add a utility to the version file module that finds the nearest version file. We probably want a struct for this so we can store the path as state and allow read / write.
Add a utility to the workspace / project abstractions to reconcile the discovered version file with the project, e.g. we can check if it's location is a prefix of the project or if it's inside the project and check compatibility with the project Python requirement.
Use in uv venv under preview mode
Use in all the new preview commands where we currently respect version files
Add a tracking issue enumerating the remaining commands

How does that sound? Kind of figuring this out as I go :)

zanieb · 2024-09-16T22:21:47Z

I'm taking this on in #6370

silvanocerza force-pushed the python-pin-workspace-subfolder branch 2 times, most recently from cd1a763 to a8452f1 Compare July 13, 2024 09:37

zanieb mentioned this pull request Jul 13, 2024

uv python pin should find .python-version file in a workspace #4971

Open

python pin now finds workspace root folder when called in subfolder

7c09387

silvanocerza force-pushed the python-pin-workspace-subfolder branch from a8452f1 to 7c09387 Compare July 14, 2024 15:07

zanieb self-assigned this Jul 14, 2024