Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add example usage to documentation, minor bugfixes to environments #906

Merged
merged 9 commits into from
Mar 20, 2023

Conversation

elliottower
Copy link
Contributor

Description

  • Added sections Installation and Usage sections in each md file to make it more obvious how to use each of them (there have been a few questions on discord which this would help with)
  • Updated RLcard envs step() functions to call render() if render_mode is set to "human" (consistent with other envs)
  • Cleaned up Butterfly manual_policy examples: supply render_mode (fixes warning when running code) and remove unnecessary render() call, and made env initialization be called first to be consistent with all other examples (group pygame and manual control code together as they are the distinct parts to the manual control examples as compared to any other example)
  • Add documentation example on Butterfly for running manual_policy example (currently the only way to find this code is by digging through the files and finding the example script, also it's not clear that there even is manual control unless you click on the individual page)
  • Cleaned up Butterfly documentation to remove prisoner example which no longer exists (was moved to tutorial)

Type of change

Documentation update

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have run pytest -v and no errors are present.
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I solved any possible warnings that pytest -v has generated that are related to my code to the best of my knowledge.
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@elliottower
Copy link
Contributor Author

elliottower commented Mar 19, 2023

Doing pytest -v the leduc holdem test currently fails but when I remove my changes (adding a simple render() call to the step function) the fail still occurs. Will look into the cause of it and try to fix it though. All other tests seem to pass.

Copy link
Member

@jjshoots jjshoots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall a very good PR. I wonder why no one else has caught the render in step catch before (myself included), and the documentation improvements are definitely much needed. There's just one minor comment --- but I presume that's to match the gymnasium way of doing things.

@@ -43,7 +43,7 @@ butterfly = ["pygame==2.1.3.dev8", "pymunk==6.2.0"]
mpe = ["pygame==2.1.3.dev8"]
sisl = ["pygame==2.1.3.dev8", "box2d-py==2.3.5", "scipy>=1.4.1"]
other = ["pillow>=8.0.1"]
tests = [
testing = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a need for this change?

@jjshoots jjshoots merged commit 626ff57 into Farama-Foundation:master Mar 20, 2023
@elliottower
Copy link
Contributor Author

Yep it was just for consistency, drove me crazy having to remember if it’s tests or testing but other repos use testing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants