[Bug] Type failure in experiment.get_metadata

[akern40]

Summary

Hello! I am trying to set up an experiment, but am running into a type error when trying to use wrap_experiment. Below is a minimal example that I'm having issue with, the error, and my specs:

Example

from garage import wrap_experiment

@wrap_experiment
def experiment(ctxt=None, seed=1):
    ...

if __name__ == "__main__":
    experiment()

Error

Traceback (most recent call last):
  File ".\src\project\experiment.py", line 32, in <module>
    experiment()
  File "C:\Users\username\.conda\envs\project\lib\site-packages\garage\experiment\experiment.py", line 368, in __call__
    ctxt = self._make_context(self._get_options(*args), **kwargs)
  File "C:\Users\username\.conda\envs\project\lib\site-packages\garage\experiment\experiment.py", line 321, in _make_context
    git_root_path, metadata = get_metadata()
  File "C:\Users\username\.conda\envs\project\lib\site-packages\garage\experiment\experiment.py", line 515, in get_metadata
    cwd=git_root_path)
  File "C:\Users\username\.conda\envs\project\lib\subprocess.py", line 411, in check_output
    **kwargs).stdout
  File "C:\Users\username\.conda\envs\project\lib\subprocess.py", line 488, in run
    with Popen(*popenargs, **kwargs) as process:
  File "C:\Users\username\.conda\envs\project\lib\subprocess.py", line 800, in __init__
    restore_signals, start_new_session)
  File "C:\Users\username\.conda\envs\project\lib\subprocess.py", line 1207, in _execute_child
    startupinfo)
TypeError: CreateProcess() argument 8 must be str or None, not bytes

Context

Context	Version
Python	3.7.10
Garage	2021.3.0
OS	Windows 10 19042.631

Fix

I believe this issue is caused by the fact that (in my context), the result of git_root_path = subprocess.check_output(('git', 'rev-parse', '--show-toplevel'), ...) is a bytes object, not a str. This then gets passed down to calls to get git_hash and git_status, which reject bytes objects for cwd. However, other parts of the code base seem to expect git_root_path to be a bytes object, which complicates matters. I'm not exactly sure if there's a single-line fix. Additionally, I'm wondering if this bug is very specific to me, since this is such a commonly-used command. Please let me know if there's an available fix for me, or if I should submit a pull request with a possible fix. Thanks!

[avnishn]

Hi @akern40. Thanks for checking out garage. Also thank you for documenting and formatting this issue so well!

A few things:

• Did you pip install garage, or are you modifying it from source? If you're pip installing it, can you tell me which version of garage you're using. If you're modifying from source, can you tell me the commit id that you have been using, of garage?

• We don't support garage directly on windows 10. I've not had a chance to look in depth at this but this issue could have to do with how underlying python primitives are executed by the windows python installation.

• You might have better luck using WSL/WSL2, which a few of our student contributors use for their development or the garage docker container also.

[krzentner]

Yeah, so git_root_path is definitely intended to be a bytes object there. I think the problem is that on Windows (only?) cwd must be a str, not bytes. This specific case shouldn't be too hard to fix (we should just write a wrapper that decodes the bytes), but I doubt this is the only Windows incompatibility. In general we don't intend to support Windows, since we don't have anyone interested in volunteering support, there's no convenient way to run the CI to make sure it works, and WSL works very well. Of course, I would still be willing to approve PR's that work around Windows compatibility problems.

[akern40]

Thanks for the responses everyone - as someone said, since WSL works so well, I unfortunately haven't had the time to really hunt down a good solution to this problem. I'm going to close this issue, but will keep in mind the notes about Windows compatibility.