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

Updated to Gymnasium v0.26.2 #72

Merged
merged 14 commits into from
Oct 21, 2022
Merged

Conversation

BolunDai0216
Copy link
Collaborator

@BolunDai0216 BolunDai0216 commented Oct 16, 2022

Description

Changed the dependency on gym to gymnasium at v0.26.2.

Fixes # (issue)

A list of the changes:

  • Changed the output of step from obs, reward, done, info to obs, reward, termination, truncation, info.
  • Removed return_info in reset() and returns obs, info all the time.
  • Added render_mode to the environments.
  • Changed the tests to use gymnasium and adapt to the new output of reset() and step().
  • Removed seed()

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • 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

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.

Could you make the suggested changes.
Is there a test that runs check_env with all of the miniworld envs ?
This is for a future pr or release where we change the randomness to use the env.np_random

@@ -128,7 +131,7 @@ def _recv_frame(self):

self.img = img

def reset(self):
def reset(self, seed, options):

Choose a reason for hiding this comment

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

Could you add type hints

Copy link
Collaborator Author

@BolunDai0216 BolunDai0216 Oct 16, 2022

Choose a reason for hiding this comment

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

Yeah. I noticed that the type hint situation is not consistent, some functions have it while others don't. Is there a guideline? Or should we just add type hints to all of the functions?

Copy link
Member

@pseudo-rnd-thoughts pseudo-rnd-thoughts Oct 17, 2022

Choose a reason for hiding this comment

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

At some point, it would be great to type hint the whole project but for now just having reset and step would be easiest. We can make another PR that adds type hinting for more of the project.

gym_miniworld/envs/remotebot.py Outdated Show resolved Hide resolved
gym_miniworld/envs/sign.py Outdated Show resolved Hide resolved
gym_miniworld/miniworld.py Outdated Show resolved Hide resolved
gym_miniworld/miniworld.py Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@BolunDai0216
Copy link
Collaborator Author

Could you make the suggested changes. Is there a test that runs check_env with all of the miniworld envs ? This is for a future pr or release where we change the randomness to use the env.np_random

test_env_checker() runs check_env() on all of the environments

@pytest.mark.parametrize("env_id", gym_miniworld.envs.env_ids)
def test_env_checker(env_id):
    if "RemoteBot" in env_id:
        return

    env = gym.make(env_id).unwrapped
    warnings.simplefilter("always")
    with warnings.catch_warnings(record=True) as w:
        check_env(env)

    for warning in w:
        if warning.message.args[0] not in CHECK_ENV_IGNORE_WARNINGS:
            raise gym.error.Error(f"Unexpected warning: {warning.message}")

    env.close()

@jkterry1 jkterry1 merged commit 8575b77 into Farama-Foundation:master Oct 21, 2022
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.

3 participants