huggingface / huggingface_sb3

Additional code for Stable-baselines3 to load and upload models from the Hub.
77 stars 23 forks source link

refactor: remove os.system call :recycle: #26

Closed robertoschiavone closed 1 year ago

robertoschiavone commented 1 year ago

According to Python docs, os.system should be replaced with subprocess module, in particular subprocess.run.

Instead of a string, subprocess.run expects a list of arguments, so that

f'ffmpeg -y -i {inp} -vcodec h264 {out}'

becomes

['ffmpeg', '-y', f'-y {inp}', '-vcodec h264', f'"{out}"']

shlex.split() takes care of automatically breaking a shell command into a sequence of arguments, so that we don't have to determine how to correctly tokenize the args manually.

Lastly, the output is printed to the stderr only to be as close as possible to the current formatting, and the generic Exception is replaced by CalledProcessError, which is thrown when the command launched by subprocess.run fails with a non-zero return code. This check is performed thanks to the parameter check=True.

simoninithomas commented 1 year ago

Thanks for the PR. We didn't add it for v2.3 (gymasnium support) but I keep it open to merge it for the future versions 🤗.