search

JuliaML / OpenAIGym.jl

OpenAI's Gym binding for Julia
Other
105 stars 20 forks source link

Add close function, remove close kw in render #26

Closed albheim closed 5 years ago

albheim commented 5 years ago

As discussed in issue #25

JobJob commented 5 years ago

Thanks!

I suppose we should at least call close(env) in the tests, even if it doesn't do much, after this line seems like an ok spot: https://github.com/JuliaML/OpenAIGym.jl/blob/08b87cb4220f2dd5cbef558f345efe14bb5d2a81/test/runtests.jl#L79

I don't think we need to add calls to render because they might not work on the CI machines.

Speaking of CI @iblis17 do you have any idea what the failures are?

iblislin commented 5 years ago

The CI failure is caused by recent Gym release. It's already fixed on their master: https://github.com/openai/gym/pull/1312

iblislin commented 5 years ago

Maybe we should fix the gym version to avoid the uncertainty from upstream. e.g.

pip install 'gym[atari]'==x.x.x
JobJob commented 5 years ago

Maybe we should fix the gym version to avoid the uncertainty from upstream.

Yes 💯

Also, I think we should test using Python 3, instead of Python 2, if possible. Python 2 people are a dying breed.

iblislin commented 5 years ago

Is PyCall python3-ready?

JobJob commented 5 years ago

Is PyCall python3-ready?

yep, I think it's python3 by default if it installs python via conda too

iblislin commented 5 years ago

ah, okay.

JobJob commented 5 years ago

I forced pushed to your master, sorry about that, should have just merged #28 rather than rebasing on top of it, but I got a little reckless not realising I had to push to your branch.

Anyway, to get back to normal on your machine you probably just want to (note this will blast away any changes you made so commit or stash them first)

git checkout master
git reset --hard 08b87cb4220f2dd5cbef558f345efe14bb5d2a81
git pull

Anyway, I added a call to close in the tests.

Thanks!