Skip to content
New issue

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

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

Already on GitHub? # to your account

Added Atari environments to tests, removed dead code #78

Merged
merged 4 commits into from
Oct 26, 2022

Conversation

Markus28
Copy link
Contributor

@Markus28 Markus28 commented Oct 26, 2022

  • Adds some atari environments to tested environments (if gym and ale are available)
  • Removed definition of minimum_testing_env_specs, which was dead code, also didn't make sense (compared specs to strings, I think)
  • Atari environments are currently not being tested for render_mode because GymEnvironment doesn't support that kwarg

Tests are currently not passing locally, which seems to be due to an unrelated problem

@Markus28 Markus28 marked this pull request as ready for review October 26, 2022 15:30
Copy link
Member

@pseudo-rnd-thoughts pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

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

Looks good to me, could you just add a comment before the atari_id to explain why though particular environments were selected.
We could replace the list with all of the environment ids in the registry that contains "Pong" just in case the number of environment expands

@Markus28
Copy link
Contributor Author

Yeah, I will add that comment :)

I also considered fetching the ids from the (old) Gym registry, but I decided against it because that registry should not really change, given that Gym is no longer being maintained. Also, I would be somewhat worried that for some reason (e.g. ale not being installed) no Atari envs show up in the registry and the test is silently skipped.

Currently, neither this test, nor test_gym_conversion are in CI, because gym isn't being installed.

@pseudo-rnd-thoughts pseudo-rnd-thoughts merged commit 24fd4a7 into Farama-Foundation:main Oct 26, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants