Repository: benchopt/benchopt
Pull request #772 (2024-09-11 05:14:46)

FIX installing pip dependencies in conda environment
The current use of `PIP_CMD` which use `sys.executable` does not point toward `python` in the right environment. As using only `pip` can cause issues on windows, I propose to create an environment file with both `conda` and `pip` deps and install everything in one go with [`conda env update`](https://docs.conda.io/projects/conda/en/latest/commands/env/update.html)

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/benchopt/benchopt/pull/772?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 82.27%. Comparing base [(`55d0f33`)](https://app.codecov.io/gh/benchopt/benchopt/commit/55d0f335d5f9d0b4775a221087eaf69ecb8d6c1d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt) to head [(`17f0030`)](https://app.codecov.io/gh/benchopt/benchopt/commit/17f0030139c9c759b2d00216f51807da8cd55fd8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #772      +/-   ##
==========================================
+ Coverage   82.23%   82.27%   +0.04%     
==========================================
  Files          46       46              
  Lines        3169     3171       +2     
==========================================
+ Hits         2606     2609       +3     
+ Misses        563      562       -1     
```

</details>
tomMoral: The issue showed up while fixing tests in scikit-adaptation/skada-bench#161

Patch:
diff --git a/benchopt/utils/conda_env_cmd.py b/benchopt/utils/conda_env_cmd.py
index 307c310..7ecb05d 100644
--- a/benchopt/utils/conda_env_cmd.py
+++ b/benchopt/utils/conda_env_cmd.py
@@ -14,7 +14,6 @@ from ..config import get_setting
 
 SHELL = get_setting('shell')
 CONDA_CMD = get_setting('conda_cmd')
-PIP_CMD = f"{sys.executable} -m pip"
 
 # On windows, calling conda without call exit the cmd script:
 # https://github.com/conda/conda/issues/12418
@@ -177,7 +176,7 @@ def delete_conda_env(env_name):
                capture_stdout=True)
 
 
-def get_cmd_from_requirements(packages):
+def get_env_file_from_requirements(packages):
     """Process the packages from requirements and create the install cmd.
 
     This detects the packages that need to be installed with pip and also
@@ -193,27 +192,27 @@ def get_cmd_from_requirements(packages):
         )
         packages = [pkg.replace(":", "::", 1) for pkg in packages]
 
-    cmd = []
     conda_packages = [pkg for pkg in packages if not pkg.startswith('pip::')]
     if conda_packages:
-        channels = ' '.join(set(
-            f"-c {pkg.rsplit('::', 1)[0]}"
+        channels = '\n  - '.join(sorted(set(
+            pkg.rsplit('::', 1)[0]
             for pkg in conda_packages if '::' in pkg
-        ))
-        conda_packages = ' '.join(
+        )))
+        channels = f"channels:\n  - {channels}\n" if channels else ""
+        conda_packages = '\n  - '.join(sorted(set(
             pkg.rsplit('::', 1)[-1] for pkg in conda_packages
-        )
-        cmd.append(
-            f"{CONDA_CMD} install --update-all -y {channels} {conda_packages}"
-        )
+        )))
+        env = f"{channels}dependencies:\n  - {conda_packages}"
+    else:
+        env = "dependencies:"
 
-    pip_packages = [
+    pip_packages = '\n    - '.join(sorted(set(
         pkg.replace("pip::", "") for pkg in packages if pkg.startswith('pip::')
-    ]
+    )))
     if pip_packages:
-        packages = ' '.join(pip_packages)
-        cmd.append(f"{PIP_CMD} install {packages}")
-    return cmd
+        env += f"\n  - pip\n  - pip:\n    - {pip_packages}"
+
+    return env
 
 
 def install_in_conda_env(*packages, env_name=None, force=False, quiet=False):
@@ -221,18 +220,25 @@ def install_in_conda_env(*packages, env_name=None, force=False, quiet=False):
     if len(packages) == 0:
         return
 
-    cmd = get_cmd_from_requirements(packages)
-    if force:
-        cmd = [c + ' --force-reinstall' for c in cmd]
-    cmd = '\n'.join(cmd)
-
-    error_msg = ("Failed to conda install packages "
-                 f"{packages if len(packages) > 1 else packages[0]}\n"
-                 "Error:{output}")
-    _run_shell_in_conda_env(
-        cmd, env_name=env_name, raise_on_error=error_msg,
-        capture_stdout=quiet
-    )
+    env = get_env_file_from_requirements(packages)
+    if DEBUG:
+        print(f"\ninstalling env packages:\n{'-' * 40}{env}{'-' * 40}")
+
+    with NamedTemporaryFile(mode='w+', prefix='env_', suffix='.yml') as f:
+        f.write(env)
+        f.flush()
+        cmd = (
+            f"{CONDA_CMD} env update -n {env_name} -f {f.name}"
+            f"{' -q' if quiet else ''}"
+        )
+
+        error_msg = (
+            f"Failed to conda install packages {packages}\nError:{{output}}"
+        )
+        _run_shell_in_conda_env(
+            cmd, env_name=env_name, raise_on_error=error_msg,
+            capture_stdout=quiet
+        )
 
 
 def shell_install_in_conda_env(script, env_name=None, quiet=False):


Test patch:
diff --git a/benchopt/tests/test_deprecation.py b/benchopt/tests/test_deprecation.py
index 7e40728..9e68874 100644
--- a/benchopt/tests/test_deprecation.py
+++ b/benchopt/tests/test_deprecation.py
@@ -1,32 +1,28 @@
 import pytest
 
-from benchopt.utils.conda_env_cmd import get_cmd_from_requirements
-from benchopt.utils.conda_env_cmd import CONDA_CMD, PIP_CMD
+from benchopt.utils.conda_env_cmd import get_env_file_from_requirements
 
 
 # TODO: remove this test in benchopt 1.7
 def test_deprecated_channel_spec():
-    with pytest.warns(DeprecationWarning):
-        cmd = get_cmd_from_requirements(["chan:pkg"])
-
-    assert len(cmd) == 1
-    assert cmd[0] == f"{CONDA_CMD} install --update-all -y -c chan pkg"
 
     with pytest.warns(DeprecationWarning):
-        cmd = get_cmd_from_requirements(["pip:pkg"])
-
-    assert len(cmd) == 1
-    assert cmd[0] == f"{PIP_CMD} install pkg"
+        env = get_env_file_from_requirements(["chan:pkg"])
+    assert env == "channels:\n  - chan\ndependencies:\n  - pkg"
 
     with pytest.warns(DeprecationWarning):
-        cmd = get_cmd_from_requirements(["pip:git+https://test.org"])
-
-    assert len(cmd) == 1
-    assert cmd[0] == f"{PIP_CMD} install git+https://test.org"
+        env = get_env_file_from_requirements(["pip:pkg"])
+    assert env == "dependencies:\n  - pip\n  - pip:\n    - pkg"
 
     with pytest.warns(DeprecationWarning):
-        cmd = get_cmd_from_requirements(["pkg1", "chan:pkg2", "pip:pkg3"])
+        env = get_env_file_from_requirements(["pip:git+https://test.org"])
+    assert env == (
+        "dependencies:\n  - pip\n  - pip:\n    - git+https://test.org"
+    )
 
-    assert len(cmd) == 2
-    assert cmd[0] == f"{CONDA_CMD} install --update-all -y -c chan pkg1 pkg2"
-    assert cmd[1] == f"{PIP_CMD} install pkg3"
+    with pytest.warns(DeprecationWarning):
+        env = get_env_file_from_requirements(["pkg1", "chan:pkg2", "pip:pkg3"])
+    assert env == (
+        "channels:\n  - chan\n"
+        "dependencies:\n  - pkg1\n  - pkg2\n  - pip\n  - pip:\n    - pkg3"
+    )
diff --git a/benchopt/utils/tests/test_conda_env_cmd.py b/benchopt/utils/tests/test_conda_env_cmd.py
index ba740d9..20da224 100644
--- a/benchopt/utils/tests/test_conda_env_cmd.py
+++ b/benchopt/utils/tests/test_conda_env_cmd.py
@@ -1,6 +1,4 @@
-import pytest
-
-from benchopt.utils.conda_env_cmd import get_cmd_from_requirements
+from benchopt.utils.conda_env_cmd import get_env_file_from_requirements
 from benchopt.config import get_setting
 
 
@@ -9,54 +7,32 @@ CONDA_CMD = get_setting("conda_cmd")
 
 def test_requirements_conda():
 
-    packages = ["dep1", "dep2", "chan1::dep3"]
-    cmd = get_cmd_from_requirements(packages)
-
-    assert len(cmd) == 1
-    cmd = cmd[0]
-
-    assert CONDA_CMD in cmd, f"Should use {CONDA_CMD} to install deps."
-    assert " dep1" in cmd, f"missing dep1 in cmd: {cmd}"
-    assert " dep2" in cmd, f"missing dep2 in cmd: {cmd}"
-    assert " dep3" in cmd, f"missing dep3 in cmd: {cmd}"
-    assert " -c chan1 " in cmd, f"missing channel in cmd: {cmd}"
-
-
-def test_deprecated_channel():
+    packages = ["dep1", "dep2"]
+    env = get_env_file_from_requirements(packages)
+    assert env == "dependencies:\n  - dep1\n  - dep2"
 
-    packages = ["chan:dep"]
-    with pytest.warns(DeprecationWarning):
-        cmd = get_cmd_from_requirements(packages)
 
-    assert len(cmd) == 1
-    cmd = cmd[0]
-
-    assert CONDA_CMD in cmd, f"Should use {CONDA_CMD} to install deps."
-    assert " dep" in cmd, f"missing dep in cmd: {cmd}"
-    assert " -c chan " in cmd, f"missing channel in cmd: {cmd}"
+def test_requirements_conda_with_channels():
+    packages = ["dep1", "dep2", "chan1::dep3", "chan2::dep4", "chan1::dep2"]
+    env = get_env_file_from_requirements(packages)
+    assert env == (
+        "channels:\n  - chan1\n  - chan2\n"
+        "dependencies:\n  - dep1\n  - dep2\n  - dep3\n  - dep4"
+    )
 
 
 def test_requirements_pip():
 
     packages = ["pip::dep1", "pip::dep2"]
-    cmd = get_cmd_from_requirements(packages)
-
-    assert len(cmd) == 1
-    cmd = cmd[0]
-
-    assert 'pip' in cmd, "Should use pip to install deps."
-    assert " dep1" in cmd, f"missing dep1 in cmd: {cmd}"
-    assert " dep2" in cmd, f"missing dep2 in cmd: {cmd}"
+    env = get_env_file_from_requirements(packages)
+    assert env == "dependencies:\n  - pip\n  - pip:\n    - dep1\n    - dep2"
 
 
 def test_requirements_mixed():
 
-    packages = ["dep1", "pip::dep2"]
-    cmd = get_cmd_from_requirements(packages)
-
-    assert len(cmd) == 2
-
-    assert CONDA_CMD in cmd[0], f"Should use {CONDA_CMD} to install dep1."
-    assert " dep1" in cmd[0], f"missing dep1 in cmd: {cmd[0]}"
-    assert 'pip ' in cmd[1], "Should use pip to install dep2."
-    assert " dep2" in cmd[1], f"missing dep2 in cmd: {cmd[1]}"
+    packages = ["dep1", "chan1::dep2", "pip::dep2", "pip::dep2"]
+    env = get_env_file_from_requirements(packages)
+    assert env == (
+        "channels:\n  - chan1\ndependencies:\n  - dep1\n  - dep2\n"
+        "  - pip\n  - pip:\n    - dep2"
+    )