-
Notifications
You must be signed in to change notification settings - Fork 284
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
Always set $EBPYTHONPREFIXES
instead of PYTHONPATH
#2887
Comments
i personally think it is a good thing that users cannot upgrade/downgrade packages that are already loaded with modules:
also, if you don't want to use the module provided version, why load the module in the first place? so i think if this is implemented it should indeed be optional |
The point is: They can. It just silently does not work. See below
Because the module might contain a dependency which is not the (primary) reason for using that module, and that might need changing. Example: TensorFlow requires a lot of additional Python packages. E.g. for 2.11 we have After loading the relevant module(s) the user creates a virtualenv and installs additional packages with Now suppose that they release a version 2.11.2 of the package with a fix we didn't anticipate when creating the easyconfig but is for some reason important for users.
A similar thing happens, when a user installs some additional python package into the virtualenv which has a dependency on a Python package contained (as a dependency/extension) in a module. But the users python package requires a different version. This has actually caused issues for us in the past which led me to "fixing" all module files. So this is not theoretically.
Probably, but the default should be to use Coming back to
Another viewpoint is: We shouldn't restrict users when we don't have to. If a user actively installs a different version of a package there might be reasons for doing so which I think are hard to anticipate in general. I mean it is nothing that can happen implicitly: The user has to actively do that. |
i see your point now, thanks for explaining.
true, but adopting the multi_deps approach was a breaking change too, so you could argue it was reverted to the original approach :) |
I think this disagreement also came up in Julia easybuilders/easybuild-easyconfigs#17455 I'm with @Flamefire here, and it was actually the major second reason for us to use One could actually use EBPYTHONPREFIXES everywhere and still have the (configurable, if we agree to disagree?) effect of overriding user installs. See this script that is installed ( # sitecustomize.py script installed by EasyBuild,
# to pick up Python packages installed with `--prefix` into folders listed in $%(EBPYTHONPREFIXES)s
import os
import site
import sys
# print debug messages when $%(EBPYTHONPREFIXES)s_DEBUG is defined
debug = os.getenv('%(EBPYTHONPREFIXES)s_DEBUG')
# use prefixes from $EBPYTHONPREFIXES, so they have lower priority than
# virtualenv-installed packages, unlike $PYTHONPATH
ebpythonprefixes = os.getenv('%(EBPYTHONPREFIXES)s')
if ebpythonprefixes:
postfix = os.path.join('lib', 'python' + '.'.join(map(str, sys.version_info[:2])), 'site-packages')
if debug:
print("[%(EBPYTHONPREFIXES)s] postfix subdirectory to consider in installation directories: %%s" %% postfix)
potential_sys_prefixes = (getattr(sys, attr, None) for attr in ("real_prefix", "base_prefix", "prefix"))
sys_prefix = next(p for p in potential_sys_prefixes if p)
base_paths = [p for p in sys.path if p.startswith(sys_prefix)]
for prefix in ebpythonprefixes.split(os.pathsep):
if debug:
print("[%(EBPYTHONPREFIXES)s] prefix: %%s" %% prefix)
sitedir = os.path.join(prefix, postfix)
if os.path.isdir(sitedir):
if debug:
print("[%(EBPYTHONPREFIXES)s] adding site dir: %%s" %% sitedir)
site.addsitedir(sitedir)
# Move base python paths to the end of sys.path so modules can override packages from the core Python module
sys.path = [p for p in sys.path if p not in base_paths] + base_paths if you change
which switches then from |
I think we should do this, is this perhaps something we could squeeze into EB5 since it would be a pretty major change? |
This could even be guided by an option. Might be hard to get always correct when custom easyblocks are involved though. Currently we have some code in a hook that replaces PYTHONPATH by EBPYTHONPREFIXES |
I'm in favor of this. Given that I used this with multideps and never say any issues, I think it should be safe to default to this.
Even if we don't cover those cases it's still worth a fix for "just" doing this in the PythonBundle and PythonPackage's, as those are most likely the stuff that the user wants to shadow with their venv anyway. |
I brought this up on the 5.0 meeting today
I'm not sure how we want to tackle this in practice though.
we either just ignore if the use is intentional, or possibly fix so that they do install into the correct sub-directory. Approach B:
and of course, we have to avoid it in all cases where the target path doesn't follow the expected site-packages directory structure. I'm leaning option towards A. Edit: Corrections, I thought about it some more, and I think it requires a hook to deal with just the easyconfigs that specify their own PYTHONPATH + having it optionally converted to EBPYTHONPREFIX so, option B is the only alternative. |
I began working on this in |
A short time ago I did a major replacement of setting
$EBPYTHONPREFIXES
instead ofPYTHONPATH
on the modules on our HPC cluster because the latter does not play nice with virtualenvs: The$PYTHONPATH
packages take precedence over the virtualenv making it impossible for users to update/downgrade packages (in their virtualenv by explicitly issuing the requests via e.g.pip
).Hence we should always set
$EBPYTHONPREFIXES
(in PythonPackage/PythonBundle) or at least provide an option to set this for each site.The text was updated successfully, but these errors were encountered: