mamba-org / gator

Conda environment and package management extension from within Jupyter
Other
260 stars 31 forks source link

Fix finding mamba on windows #135

Closed ericpre closed 3 years ago

ericpre commented 3 years ago

The following fails on windows:

from subprocess import PIPE, Popen

which = "where"

process = Popen(
    [which, "mamba"], stdout=PIPE, stderr=PIPE, encoding="utf-8"
)
output, error = process.communicate()

if process.returncode != 0:
    raise RuntimeError(error)

mamba_exe = output.strip() or "mamba"

process = Popen(
    [mamba_exe, "--version"],
    stdout=PIPE,
    stderr=PIPE,
    encoding="

with the error:

Traceback (most recent call last):

  File "<ipython-input-1-2076a85bac6b>", line 15, in <module>
    process = Popen(

  File "C:\Users\Eric\HyperSpy-bundle\lib\site-packages\spyder_kernels\customize\spydercustomize.py", line 108, in __init__
    super(SubprocessPopen, self).__init__(*args, **kwargs)

  File "C:\Users\Eric\HyperSpy-bundle\lib\subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,

  File "C:\Users\Eric\HyperSpy-bundle\lib\subprocess.py", line 1311, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,

FileNotFoundError: [WinError 2] The system cannot find the file specified

Because output is 'C:\\Users\\Eric\\HyperSpy-bundle\\Scripts\\mamba.exe\nC:\\Users\\Eric\\HyperSpy-bundle\\condabin\\mamba.bat\n'

I have tried to add a test but I don't understand how the test suite works.

github-actions[bot] commented 3 years ago

Binder :point_left: Launch a binder notebook on the branch _ericpre/gator/fix_finding_mambawindows

fcollonval commented 3 years ago

Good catch @ericpre - thanks a lot for that.

To make it more robust, I propose to change a bit the following code:

-                which = "which"
+                cmd = ["which", "mamba"]
                 if sys.platform == "win32":
-                    which = "where"
+                    cmd =["where", "mamba.exe"]

                 process = Popen(
-                    [which, "mamba"], stdout=PIPE, stderr=PIPE, encoding="utf-8"
+                    cmd, stdout=PIPE, stderr=PIPE, encoding="utf-8"
                 )

So we are sure to capture an executable on Windows and not the shell script.

Regarding testing, the tests are stored in the mamba_gator/tests folder. I would advice creating a new file for this one like test_manager.py (it must start with test_).

Then in that file, the test functions are all function named def test_...(). I would suggest instantiating a EnvManager instance and test the property manager; something like:

def test_EnvManager_manager():
    try:
        import mamba
    except ImportError:
        expected = "conda"
    else:
        expected = "mamba"

    manager = EnvManager("", None)

    assert Path(manager.manager).stem == expected

Let me known if you have questions.

ericpre commented 3 years ago

Thanks, your test is god and simple! My issue was mainly with getting a EnvManager instance working. I have tried to do something similar to https://github.com/mamba-org/gator/blob/96a711f9ef3aa03e488bd14b4322c750b5e88c55/mamba_gator/tests/test_api.py#L606-L613 for mamba and it was not working.