-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
Updates the make and registration functions with better parameters and hints #3041
Updates the make and registration functions with better parameters and hints #3041
Conversation
@RedTachyon I have readded / kept the make overloads. I investigated testing that the overloads were correct. def test_make_overloads():
env_make_hints = {}
for overloaded_func in typing_extensions.get_overloads(gym.make):
make_type_hints = typing_extensions.get_type_hints(overloaded_func)
assert (
"id" in make_type_hints and "return" in make_type_hints
), f"Missing type hints for overloaded `gym.make` id and return, {make_type_hints.get('id', 'unknown env')}"
if not isinstance(make_type_hints["id"], type):
for env_id in typing_extensions.get_args(make_type_hints["id"]):
obs_type, act_type = typing_extensions.get_args(
make_type_hints["return"]
)
env_make_hints[env_id] = (obs_type, act_type)
assert (
env_make_hints.keys() == gym.envs.registry.keys()
), f"Missing the make overloads envs: {set(gym.envs.registry.keys()) - set(env_make_hints.keys())}. Additional envs: {set(env_make_hints.keys()) - set(gym.envs.registry.keys())}"
# todo: add type check on the obs and action type |
# Conflicts: # tests/envs/test_make.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good apart from a minor change
tests/testing_env.py
Outdated
@@ -49,7 +49,7 @@ def __init__( | |||
render_modes: Optional[List[str]] = None, | |||
render_fps: Optional[int] = None, | |||
render_mode: Optional[str] = None, | |||
spec: EnvSpec = EnvSpec("TestingEnv-v0"), | |||
spec: EnvSpec = EnvSpec("TestingEnv-v0", None), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this should also be "no-entry-point"?
@pseudo-rnd-thoughts would it make sense to make a string constant, say, |
Yes that probaby would have made sense but this is purely in testing and this file alone I believe |
This PR originally started with fixing the type hints for
EnvSpec
entry_points where the value could be None by default however this won't work.After this I looked through the whole file and updated / removed the following sections
entry_point
inEnvSpec
__relocated__.py
file and related code. This error seems old and needed removing at some pointmake
overloads with all environments.EnvRegistry
as noted to be removed at 1.0EnvSpec
attributes as parameters togym.register