-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Adds option for conda environments to ignore Python's user site packages #13635
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
base: main
Are you sure you want to change the base?
Adds option for conda environments to ignore Python's user site packages #13635
Conversation
CodSpeed Performance ReportMerging #13635 will not alter performanceFalling back to comparing Summary
|
@@ -122,4 +123,7 @@ def execute(args: Namespace, parser: ArgumentParser) -> int: | |||
dry_run=False, | |||
) | |||
|
|||
return install(args, parser, "create") | |||
install(args, parser, "create") |
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.
Removing this return statement won't do anything bad. The install
function does not return any return codes. Just None
if successful.
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.
The return type in the signature needs updating, or we do need to return 1
in case of errors, I think. Better double check because this is the type of oversight that ends up hitting CLI usage with unreported errors.
I think it's better to implement this as a conda package instead of as yet another configuration option. For eg: see https://github.com/conda-forge/conda-ecosystem-user-package-isolation-feedstock |
Thanks for linking to your project. I personally do not think packages should be used as configuration, but I know from past conversations that there is already a precedent set for this in the conda community. I think this pull request makes this option much more discoverable (e.g. it's inclusion in the conda documentation) and easier to use (no additional packages to install). Yes, we are adding another configuration option to conda, but I think this penalty is worth the benefits to ease of use. If we were going to choose the package route, I would at the very least like this to be documented somewhere clearly in the conda documentation so that our users would have a chance to find it. @itcarroll, do you have any thoughts on this? |
I had the same thought originally, @isuruf, and proposed an improvement on the package you linked. Discussion there and further consideration convinced me this doesn't feel good as a package. Either 1) you would have some "isolate-me" package in each environment you want isolated, or 2) you would have an "isolate-on-create" package in the condo base environment. With 1, you introduce an oddball dependency having nothing to do with the needs of the project the environment was built for. With 2, you still need a command line option so you could disable isolation in particular cases. Generally, I don't think it's a problem to have a plethora of well-documented configuration options for conda. I believe the very early resistance to solving #13337 was due to the necessary configuration being Python specific, not to it being a configuration option somewhere. |
@@ -0,0 +1,19 @@ | |||
### Enhancements | |||
|
|||
* Adds a new setting, `isolate-python-env`, that can be used to disable Python programs in a conda environment from using the user site packages directory. (#13635) |
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.
I thought we were trying to stay away from adding language-specific options in conda
itself now, and instead resorting to plugins. That's what prompted the inclusion of EXTERNALLY-MANAGED
in conda-pip
, at least, and I wonder if that project would be a better home for this type of contribution.
If that's not a good idea, then I'd lean towards having a setting that is a bit more abstract, like environment-isolation
, that accepts a series of "features" to enable, one of them being pyvenv.cfg
.
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.
That's the conflict of concept that I fear as well. Venv-style environment management is part of the conda core experience and has an outsized impact on how pip (etc) install into it.
I outlined a possible change of policy in my comment here: #13337 (comment)
Could we tweak this solution here, that when conda-pip becomes ready to roll out to users (as a default dependency for the installers), it can become the package responsible for isolating conda environments from Python site-packages? Or would that overstep the role of plugins given that environments are a core aspect of conda?
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.
Is the end goal to allow that kind of configuration once the behavior has changed, or will it eventually be frozen? Asking in case we want to start with a pseudo-private name like _include_system_*
or similar.
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.
[conda-pip] can become the package responsible for isolating conda environments from Python site-packages
Does that imply that an environment has to have pip (or conda-pip) in order to be isolated? Pip is not a requirement for isolation of venv environments, and consistency is nice.
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.
[conda-pip] can become the package responsible for isolating conda environments from Python site-packages
Does that imply that an environment has to have pip (or conda-pip) in order to be isolated? Pip is not a requirement for isolation of venv environments, and consistency is nice.
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.
@jaimergp, I would be happy to re-write this as an environment_isolation
setting that would accept a list of strings, pyvenv
being one of them.
@itcarroll, would this suffice?
@jezdez, I don't think it makes sense to make it only part of the conda-pip
package based on what @itcarroll has said.
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.
Changing to environment_isolation
seems okay, even though I can't think of another package (other than python) that could benefit. I could see that someone might want the equivalent of a $PATH that only looks in the environment, as another form of isolation. So perhaps environment_isolation
would be a conda feature and not just a python feature.
Hi there, thank you for your contribution! This pull request has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs. If you would like this pull request to remain open please:
NOTE: If this pull request was closed prematurely, please leave a comment. Thanks! |
Checked |
Fixes: #13337
Description
Please see the issue above for more background on why this is being done.
This pull request introduces a new
include_python_user_packages
configuration setting that can be set via a CLI option, environment variable or config file. When set tofalse
, conda will create apyvenv.cfg
in the root of the conda environment with the following contents:include-system-site-packages = false
The presence of this file will disable Python programs from looking in the user site packages location for additional libraries that can be imported.
This new configuration variable will default to
false
to reflect how conda currently behaves and to avoid breaking environments which have relied on this behavior in the past.Checklist - did you ...
news
directory (using the template) for the next release's release notes?