-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Enhancement: Change order of discovery of python environments for uv pip install
#7124
Comments
Hm. I think you have a fair point but we can't tell if it's a conda base environment or if it's an actual active conda environment, right? If it's not the base environment I would expect us to prefer it over Note this would be a breaking change. |
I thought about that too and I think there's an argument to be made for it either way. For consistency, I personally think it is easier to remember it as " By changing the order, if a user really wants to install it in |
If I am understanding the issue correctly I don't think this matters, virtual environments can be children of conda environments, but not the otherway around. If I am using both conda and venv my typical worfklow would be:
I would expect that final |
Wait, does sourcing a virtual environment always create the env variable In particular if I run:
And there happens to be a ".venv" in my folder, I would expect uv to install in the conda environment, not in the the virtual environment. Perhaps print a warning that ".venv" folder was detected but not activated, so installing in conda environment. In general, if a user is using conda, I would have an expectation that they want to install into conda environments, unless a virtual environment was explicitly activated. So going back to the "base" environment issue, IMO conda should solve this on their side my adding an |
Can I ask when do you expect a user to do both If someone explicitly does choose to create a Additionally, I can imagine users wanting to install I guess my point is that if there's a |
When I want to create a virtual environment for a specific project, but have it based on the Python versions created by conda. This can happen either because of how some dependencies are managed, or because some tooling/testing works well with virtual environments but not conda environments but I can still use conda to create the binaries (such as Python executable, openssl, etc.), I have done this a lot in the past for pip test suite.
The opposite conclusion can be reached with similar reasoning, I'm cding around, I remember I need to install something in my conda environment, I activate it, I install with uv, and to my surprise it doesn't get installed in my conda environment when I try and run
I do think uv should be explicit about if this is always true or not, even if it is the default behaviour, I think there should be some option to install into my conda environment regardless of if there is a |
Fair points. Accidentally installing into |
I guess this really goes back to
If we can tell, we can have better behavior. |
You could manually add the
btw, is there a plan to try and locate all environments and the details from environment managers like conda? (I think https://github.com/microsoft/python-environment-tools attempts to do this). Then you would know if it's the base environment because it would be the conda environment either called "base" or "root". |
Naw, we don't intend to read other environment managers or tools at this time. I think it's better to read from standard locations. |
Given that there's not an easy way to tell the difference, and without special casing |
Thinking on it this would directly affect my tooling which does the following:
If the order is changed, and the user happens to have created a virtual environment in the directory they run the script, then it would keep installing the requirements for each environment in the venv. So, while I'm not against changing the ordering, I think it should be made a very explicit change, with an option to ignore the local ".venv". |
We're pretty unlikely to change the order. This is the first complaint we've gotten and the behavior has been this way for a while. Furthermore, active environments always take precedence over |
Can you clarify what is the project interface? |
Like these commands: https://docs.astral.sh/uv/getting-started/features/#projects |
I see, thanks for sharing. I will defer to your judgement on this, you definitely will have a better sense of what users will likely want. I do want to make two counter arguments though:
|
FWIW, it appears you can detect if the base environment is currently activated if https://conda.io/projects/conda/en/latest/dev-guide/deep-dives/activation.html#conda-activate If someone wants to make a seperate issue about uv avoiding installing into conda base environments as though they were system versions of Python? |
Nevermind, I just made it: #7137 |
Thanks @notatallshaw! I wonder if we should close this in favor of that? |
Thinking out loud here. Anaconda let's the user customize the installation location (on Windows it defaults to It seems that the conda philosophy is to not mutate/pollute the system path or use environment variables and instead it utilizes the current shell context by modifying the shell startup scripts via the I think that calling/shelling out to a
(base) PS C:\Users\me> conda env list
# conda environments:
#
base * C:\ProgramData\anaconda3 This would probably be good enough, but as an extra precaution it could be cross-referenced with the (base) PS C:\Users\me> conda info
active environment : base
# ...
base environment : C:\ProgramData\anaconda3 (read only)
# ... Note that the conda base environment is marked as read-only in the output of |
The output of let conda_info_pattern = r"base environment\s*:\s*(.+?)(?:\s*\(read only\))?\s*$";
let conda_env_list_pattern = r"^\s*\*\s+([^\s]+)\s+([^\s]+)"; |
Maybe a helper function to do this can look something like this? /// Checks if the current conda environment is the base environment.
///
/// This function runs `conda info` and `conda env list` commands asynchronously,
/// captures the relevant information using regular expressions, and compares
/// the base environment path with the active environment path.
///
/// # Returns
/// - `Ok(true)` if the current environment is the base environment.
/// - `Ok(false)` if the current environment is not the base environment.
/// - `Err` if there is an error running the commands or processing the output.
async fn is_conda_base_env() -> Result<bool, Box<dyn Error>> {
let base_env_pattern = Regex::new(r"base environment\s*:\s*(.+?)(?:\s*\(read only\))?\s*$")?;
let active_env_pattern = Regex::new(r"^\s*\*\s+([^\s]+)\s+([^\s]+)")?;
let (conda_info_output, conda_env_list_output) = try_join!(
Command::new("conda").arg("info").output(),
Command::new("conda").arg("env").arg("list").output()
)?;
Ok(
base_env_pattern
.captures(&String::from_utf8_lossy(&conda_info_output.stdout))
.and_then(|caps| caps.get(1).map(|m| m.as_str().trim()))
== active_env_pattern
.captures(&String::from_utf8_lossy(&conda_env_list_output.stdout))
.and_then(|caps| caps.get(2).map(|m| m.as_str()))
)
} |
Thanks for digging into this. I'm very hesitant to shell out to conda, it seems brittle and slow. Do you some data about how long that invocation takes? |
I don't, just hacked this together real quick. Not really sure what the best entrypoint for this check could be or how to go about caching the conda environment state. Probably way better ways to do this. |
Environment variables might be a lot faster. Seems like conda has quite a few of them so there's probably more than one way to get the correct paths. This page shows a lot of the environment variables that conda uses: https://conda.io/projects/conda/en/latest/dev-guide/deep-dives/activation.html#activation-deactivation-scripts /// Checks if the current conda environment is the base environment.
///
/// This function reads the `CONDA_PREFIX` and `CONDA_DEFAULT_ENV` environment variables,
/// extracts the last component of the `CONDA_PREFIX` path, and compares it with `CONDA_DEFAULT_ENV`.
///
/// # Returns
/// - `true` if the current environment is the base environment.
/// - `false` if the current environment is not the base environment or if either environment variable does not exist.
fn is_conda_base_env() -> bool {
if let (Ok(conda_prefix), Ok(base_env)) = (env::var("CONDA_PREFIX"), env::var("CONDA_DEFAULT_ENV")) {
if let Some(current_env) = Path::new(&conda_prefix).file_name().and_then(|s| s.to_str()) {
return current_env == base_env;
}
}
false
} |
I would concur, from yy experience wrapping conda, particularly on Windows with lots of security tools installed, you can be talking about a few seconds to get a response back. |
Hm This might work actually..
|
I think I have the right idea in #7691 |
That's just #7137 with the suggestions of env vars I made there right? |
Sorry I forgot about that issue and was riffing off of @chrisrodrigue's function. It is similar to your suggestion there, though the heuristic is a little different. I'm not sure which approach is better. |
As long as you don't make any assumptions about the relationship of the paths between the base environment and the other environments you should be fine. As users can both change where non-base environments are created, and in fact manually specify the directory for them on each creation. |
Basically the assumption is that a base environment is always named "root" or "base" and doesn't have a prefix path ending in the name. |
This is probably fine, but be aware nothing stops a user installing their conda software into a directory called "base", e.g. |
Is Could an arbitrary user created conda env called base be exploited in this context? |
I did a bit of investigation and found it doesn't do what I think, so I take back my suggestion for using What I found was there are a series of
This is problematic to use as an indication of what is base then because the user can activate base again via |
Good find! I wish the conda environment variable documentation were a little more verbose so that we didn’t need to experiment 😅 |
From https://docs.astral.sh/uv/pip/environments/#discovery-of-python-environments
I believe the implementation should be changed to check for the conda environment at the very end, i.e. this is the order it should operate in:
This is because most users that have installed conda will have auto activate base enabled, since that is the default. For example:
This means when someone runs
uv pip install
even with a.venv
folder in the current directory, it'll install all the dependencies into the conda environment, which in my opinion is not what users would want. If a.venv
folder doesn't exist, I think it is appropriate to install intoCONDA_PREFIX
.I made this mistake and accidentally installed all the project dependencies into my base conda environment environment.
The text was updated successfully, but these errors were encountered: