-
Notifications
You must be signed in to change notification settings - Fork 792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create virtualenv by default in pip install
and pip sync
#2665
Conversation
@@ -132,7 +142,7 @@ fn missing_venv() -> Result<()> { | |||
----- stdout ----- | |||
|
|||
----- stderr ----- | |||
error: failed to read from file `requirements.txt` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird, this test was passing but with the "wrong" failure.
...I feel like I'd find it quite annoying if I had a virtual environment already in a project saved in an Could there maybe be a quick "Create and activate a new virtual environment? [y/n]" prompt before creating one and going ahead with the installation? |
Is it really so bad if |
It's going to create me more hassle though, right, because now I have two virtual environments when I only need one, so now I have to delete one of them? |
Yes! But today, you have to re-run two commands anyway in this failure path. And this way, it's (1) right some of the time, but more importantly (2) encourages users to follow the happy path. |
Today if I'm in a repo locally on the command line, I run
With this PR, I'd have to do the following:
That's one, possibly two more steps by my reckoning ;)
I agree, and I like the idea of making this easier! I'm just asking that we refuse the temptation to guess, and ask users to type "y" to confirm that this is really what they wanted before we proceed |
I'm not a fan of interactive prompts at all. It sounds like what you want is #1422 :) |
I'm not a fan at all of tools guessing what the user wanted to do when it's not explicit :/
eh, I'm not sure that's what I want. I actually only learned today that But I know we've had that behaviour for a while now. Perhaps I'm being too curmudgeonly :) |
:) Yeah we've gone back and forth on this. If we think know what you want to do should we just do it? Should we prompt? Should we error and tell you how to fix it? I think we should have a consistent policy here but we haven't come to one yet. @MichaReiser has had a different opinion than me in the past so it might be worth hearing what he thinks. I think that we want to hide the virtual environment abstraction — things should "just work" in a project directory. We also want to innovate on the idea that "virtual environments are disposable and free". Previously, they were not free and that they are is a big part of our value proposition. What happens now that they are cheap? How can workflows evolve? What can we do to push boundaries based on this idea? I find these questions motivating me to explore things I would otherwise find surprising.
I find this a very strong argument for the behavior proposed in this pull request. It's a continuation of our existing philosophy. Whether or not we should have done that in the first place is another discussion I guess. |
writeln!(printer.stderr(), "Activate with: {}", activation.green())?; | ||
} | ||
|
||
commands::venv::create(&path, interpreter, Prompt::default(), false)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted by Damian, we should definitely not create this when --dry-run
is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a test case with pip install --dry-run
I'll also add maybe I use conda environments, not virtual environments, to install my packages. uv will always do the wrong thing in that case. As I said in the issue, as a non beginner I would definitely want some way to switch the behavior back, I would want it to error. |
Yeah, this wouldn't happen when you have a Conda environment active, just as we don't error today if you have a Conda environment active. |
Sorry I wasn't clear, the scenario I was thinking of is you have conda installed, but you have set conda to not activate any conda environment by default (not even base), and you use uv to install. uv will not see an activated conda environment, so presumably will create a virtual environment, but as the user only uses conda environments this will be the wrong behavior for the user. This was another example where I would like to be able to set a switch somewhere for uv to not do this. |
I'm personally a big fan of prompts, because they force the user to be explicit about what they wanted, and you generally only have to hit a single key on the keyboard to get the behaviour that you were probably trying to get all along :)
I love that as a goal, and I'm very much in favour of us trying to innovate new workflows for Python packaging. But right now, where the virtual environment is located seems pretty relevant and unabstracted. You still need to run
Yeah, agreed that it's consistent with some of the decisions that have already been taken. It does feel like another step down that road, though |
Btw, it might actually be very costly to write to the currently directory structure, the user could have set their current directory to a magnetic tape device, or a NAS that's on the opposite side of the world, or a write once disk. I'm pro the defaults being beginner friendly, I would just like users to have an option to turn them off once they are burnt by those defaults. |
The experience i'm interested in is one where you git clone and |
Also, as you're picking a single color. When I'm testing a open source project I often have to create 5 virtual environments, one for each major version of Python the project supports. |
I agree with the goal of attempting to standardise virtual environment names somewhat, and I think For me personally, it would not be possible for me to call all my virtual environments |
I don't think I have a strong opinion on this. To me this seems like a logical next step from how My only concern, but that's a pre-existing one with how I feel like a middle ground here would be if we have our own configuration file and uv could be configured to opt in of this behavior. That might still be confusing because said configuration is unlikely to exist in CI |
I wrote it before in other discussions, but let me rehash it here again. If I may suggest - I think it's quite bad idea to create such venv dynamically in current working directory. This might lead to a numer of problems and a little magical experience for the user - where for example they can end up with multiple such venvs created in different directories semi-randomly and where various IMHO it's a good idea to create such venv dynamically, but it should be created in a well-known place in the HOME directory (or equivalent on windows). My initial proposal was to use the same directory that is currently used by I think also that it is very much in line with the spirit of PEP-0370 - the whole reason for the PEP:
I can't think of any bad side effect of choosing such venv as the "default" one (and BTW. I already applied it for the "production" image of Airflow where we install airflow in My 3 cents. |
Also for some cross reference the other suggestion to suport If you go the path I propose you will simply treat all installations that have no venv nor Of course it wwould have to be somehow synchronized with But of course - those are just suggestions, and feel free to ignore them @charliermarsh :). |
This is the wrong behaviour, it should look for a And "activating" is a anti-pattern that creates state in a single shell-window.
Yes, the location of all your executables change, breaking everything else downstream. I have over 100 venv-based app on my system,
would become with uv, with zero benefit, and many disadvantages:
So now my users see an empty dir in /opt/venv-xxx. When supporting junior devs or non-technical users, |
I’m not sure how you came to this conclusion but it seems like a significant exaggeration, and candidly the use of phrasing like this makes me less likely to spend time working through the rest of your comment. I don’t think this change has anything (?) to do with where your binaries get stored and your ability to use multiple disparate virtual environments vis-a-vis main. It changes almost nothing vis-a-vis main, apart from that what was previously an error would now fallback to a different behavior. So any workflows that “work” today would not be impacted at all. I’m not planning to make this change now given how strongly people have replied here (I appreciate the input, thanks all) so I likely won’t spend much time debating it, but we may revisit in the future. |
Late to the party, but this discussion is definitely worthy of the classic xkcd. I liked @AlexWaygood suggestion on prompting (I think it benefits both sides of the argument). We'd prompt the user if one isn't found in the predetermined locations and ask if they'd like for uv to create it for them with This guides a beginner user into understanding venv's and the crucial role they play in the ecosystem (no magic). Similarly, this prompting could be disabled via an env var or if For perspective, poetry already creates venvs by default unless you set |
Closes #2661.