Skip to content
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

enhance Conda easyblock: add support for specifying list of conda packages and Python version to install + using mambaBetter conda support #2992

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

xerbalind
Copy link

No description provided.

@@ -88,12 +127,24 @@ def install_step(self):
cmd = "%s conda create --force -y -p %s %s" % (self.cfg['preinstallopts'], self.installdir, install_args)
run_cmd(cmd, log_all=True, simple=True)


def det_python_version(self):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too many blank lines (2)

self.log.info("Installed conda requirements")

run_cmd("%s install -p %s --no-deps %s" % (conda_cmd,self.installdir,install_args) ,log_all=True,simple=True)
run_cmd("ln -sf $(which python) %s" % (os.path.join(self.installdir,"bin","python")))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing whitespace after ','


self.log.info("Installed conda requirements")

run_cmd("%s install -p %s --no-deps %s" % (conda_cmd,self.installdir,install_args) ,log_all=True,simple=True)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (121 > 120 characters)
missing whitespace after ','
whitespace before ','

run_cmd(cmd, log_all=True, simple=True)

install_args = ""
if isinstance(self.src,list):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing whitespace after ','


python_version = self.cfg['python_version'] or f"python{self.det_python_version()}"

cmd = "%s %s create --force -y -p %s python=%s --no-default-packages --no-deps" % (self.cfg['preinstallopts'], conda_cmd, self.installdir,python_version)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (165 > 120 characters)
missing whitespace after ','

'source_urls': [base_url + 'noarch',base_url + 'linux-64']
})

self.cfg.update('sources',sources)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing whitespace after ','

base_url = f"https://conda.anaconda.org/{package['channel']}/"
sources.append({
'filename': package['fn'],
'source_urls': [base_url + 'noarch',base_url + 'linux-64']
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing whitespace after ','


sources = []
for package in self.cfg['conda_packages']:
base_url = f"https://conda.anaconda.org/{package['channel']}/"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiple spaces after operator

@@ -39,6 +39,22 @@
class Conda(Binary):
"""Support for installing software using 'conda'."""

def __init__(self,*args,**kwargs):
super(Conda,self).__init__(*args,**kwargs)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing whitespace after ','

@@ -39,6 +39,22 @@
class Conda(Binary):
"""Support for installing software using 'conda'."""

def __init__(self,*args,**kwargs):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing whitespace after ','

@boegel boegel added this to the 4.x milestone Sep 6, 2023
@boegel boegel self-assigned this Sep 6, 2023
@ocaisa
Copy link
Member

ocaisa commented Sep 6, 2023

I think we may need some additional changes in the final module file for a conda package. We should consider:

  • Should each conda package should have a family attached so that you cannot have multiple loaded at once? In the current approach if you have two loaded, do they interfere with each other (since the second module will overwrite CONDA_ENV/CONDA_PREFIX/CONDA_DEFAULT_ENV , see here)?
  • Should we conflict with GCCcore? This approach was suggested by @branfosj so that a conda installation is never used as a dependency (since it is extremely likely to interfere with the build)
  • Currently the final module sets PKG_CONFIG_PATH and I think this might be a mistake as packages may pick up on whatever this contains.

@boegel
Copy link
Member

boegel commented Sep 6, 2023

I'll look into cleaning this up, and making sure the changes are backwards compatible.

@xerbalind was working in the HPC-UGent team as summer intern in Aug'23, and opened this WIP PR on my request to ensure his efforts were not lost.

@ocaisa
Copy link
Member

ocaisa commented Sep 8, 2023

@boegel There are some straightforward improvements to the conda easyblock in #2996 (which also covers the mamba use case in a more generic way) that I think I will merge once I've added some test reports. Since you'll take over here I think it'll be ok to do that and then you can integrate that into this branch.

@boegel boegel changed the title Better conda support enhance Conda easyblock: add support for specifying list of conda packages and Python version to install + using mambaBetter conda support Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants