Closed paulhendricks closed 7 years ago
That looks interesting! The code appears simple enough to maintain. I would be fine with moving it if you are.
To be a little greedy I would ask for tests and an accurate REQUIRE file (Requests.jl with its min version) in order to spot problems using travis and appveyor
I would support a move, with the qualification that the recently moved OpenAIGym.jl is the "official JuliaML wrapper" since that uses LearnBase/Reinforce abstractions (ie. not a direct wrapper).
It would be good to add a note to that effect in the readme, and to update the website accordingly.
On Sat, Mar 4, 2017 at 11:47 AM Christof Stocker notifications@github.com wrote:
That looks interesting! The code appears simple enough to maintain. I would be fine with moving it if you are.
To be a little greedy I would ask for tests and an accurate REQUIRE file (Requests.jl with its min version) in order to spot problems using travis and appveyor
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaML/Roadmap.jl/issues/16#issuecomment-284163812, or mute the thread https://github.com/notifications/unsubscribe-auth/AA492swTYkk6VH_tpa4vRnyhEs6vZ1c3ks5riZWtgaJpZM4MTIOO .
Both sound good to me! I'll update accordingly and submit a transfer request.
@Evizero @tbreloff
Do either of you guys have any ideas/resources on how to set up / tear down a Python server for unit tests? If not, I'll keep working at it until I figure it out but I figured it was worth asking.
I don't quite know, but I am guessing one does it in one of two ways
PyCall.jl
/Conda.jl
for the tests.@Evizero @tbreloff
I've updated https://github.com/paulhendricks/OpenAIGymAPI.jl/ with the updates requested. I'm going to add a couple more unit tests once I get them working. Once I do that, are you guys okay with a transfer request?
Looking good! yes, sure.
A bit of feedback:
Those are certainly good goals but I don't think they should hold up a merge/transfer.
On Thu, Mar 9, 2017 at 9:07 AM Christof Stocker notifications@github.com wrote:
Looking good! yes, sure.
A bit of feedback:
- Please do make sure that you cover all the package functionality with the tests (so the coverage should be around 100 %). Naturally it is not about testing the OpenAI gym itself. just that your package does what it is supposed to and all the julia code executes as expected.
- If the package doesn't support windows (or the tests don't run on windows/appveyor), please reflect that with the badges
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaML/Roadmap.jl/issues/16#issuecomment-285360100, or mute the thread https://github.com/notifications/unsubscribe-auth/AA492tjR0r23iKAfsQjzGlYCY9ahK0nKks5rkAemgaJpZM4MTIOO .
I leave it to you, but we should aim at keeping the packages we host easily maintainable, since the number keeps growing
Awesome feedback, thanks. I got the code coverage up to about 97% and there's still a little bit of testing and implementing to be done but it's trivial.
I'm struggling to get the Windows and Mac builds working but I'm sure I can figure that out.
I'll submit a transfer once everything above is cleared up!
@tbreloff @Evizero I just transferred. I'll continue to maintain this as the OpenAI REST API updates, and will continue to try to get the Windows AppVeyor build to work.
If all is well and good I think this issue can be closed!
Glad to hear! Thanks for the effort
@tbreloff @Evizero
Would there be any interest in moving OpenAIGymAPI.jl into JuliaML? Instead of calling OpenAI Gym directly it provides a wrapper around their API.
https://github.com/paulhendricks/OpenAIGymAPI.jl