envs(envhub): add optional kwargs support to make_env
#2599
Conversation
jadechoghari
added
simulation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
There was a problem hiding this comment.
Pull request overview
This PR adds support for passing optional keyword arguments (**kwargs) to hub environments' make_env functions, enabling users to customize environment configurations when loading from the Hugging Face Hub. This enhancement allows users to pass parameters like config_path, config_overrides, or any custom configuration to hub environments without modifying the core API.
Key changes:
- Extended
make_env()API to accept and forward arbitrary keyword arguments to hub environments - Updated documentation with examples showing how to use kwargs for custom configurations
- Added comprehensive tests verifying kwargs are correctly passed through the call chain
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/lerobot/envs/factory.py | Added **kwargs parameter to make_env function signature and forwarded kwargs to _call_make_env |
| src/lerobot/envs/utils.py | Updated _call_make_env to accept and forward **kwargs to hub's make_env, updated error message and docstring |
| tests/envs/test_envs.py | Added comprehensive test test_make_env_from_hub_with_kwargs covering config_path, config_overrides, and custom kwargs |
| docs/source/envhub.mdx | Updated documentation to show **kwargs in function signatures, added new section with examples, and updated imports |
Comments suppressed due to low confidence (1)
docs/source/envhub.mdx:109
- The docstring should be updated to document the new
**kwargsparameter. The Args section should include an entry explaining what kwargs are accepted and how they're used.
"""
Create vectorized environments for your custom task.
Args:
n_envs: Number of parallel environments
use_async_envs: Whether to use AsyncVectorEnv or SyncVectorEnv
Returns:
gym.vector.VectorEnv or dict mapping suite names to vectorized envs
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert hasattr(env, "hub_config") | ||
| assert env.hub_config["config_path"] == "/path/to/config.yaml" | ||
| env.close() | ||
|
|
||
| # Test with config_overrides dict | ||
| envs_dict = make_env( | ||
| hub_id, | ||
| n_envs=1, | ||
| trust_remote_code=True, | ||
| config_overrides={"scene.object": "microwave", "sim.dt": 0.01}, | ||
| ) | ||
| env = envs_dict["cartpole_suite"][0] | ||
|
|
||
| assert env.hub_config["config_overrides"]["scene.object"] == "microwave" | ||
| assert env.hub_config["config_overrides"]["sim.dt"] == 0.01 | ||
| env.close() | ||
|
|
||
| # Test with arbitrary extra kwargs | ||
| envs_dict = make_env( | ||
| hub_id, | ||
| n_envs=1, | ||
| trust_remote_code=True, | ||
| custom_param="value", | ||
| another_param=42, | ||
| ) | ||
| env = envs_dict["cartpole_suite"][0] | ||
|
|
||
| assert env.hub_config["extra_kwargs"]["custom_param"] == "value" | ||
| assert env.hub_config["extra_kwargs"]["another_param"] == 42 | ||
| env.close() |
There was a problem hiding this comment.
There's an inconsistency in how kwargs are expected to be stored in the hub environment. Lines 284-285 expect config_path to be at env.hub_config["config_path"], while lines 311-312 expect custom kwargs to be at env.hub_config["extra_kwargs"]["custom_param"]. This suggests that config_path and config_overrides are treated as special top-level kwargs, while other kwargs go into extra_kwargs. However, this distinction is not documented or explained in the test, making it unclear what the expected behavior is for the dummy hub environment. Consider adding a comment explaining this structure or ensuring consistency in how kwargs are stored.
| from pathlib import Path | ||
| from typing import Any |
There was a problem hiding this comment.
[nitpick] The imports Path and Any are added but not used in this example. These imports should either be removed or the example should be updated to demonstrate their usage (e.g., showing how to use config_path parameter with Path).
| from pathlib import Path | |
| from typing import Any |
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| def make_env(n_envs: int = 1, use_async_envs: bool = False): |
There was a problem hiding this comment.
The example make_env function signature is missing the **kwargs parameter that is now expected. The function signature should be updated to def make_env(n_envs: int = 1, use_async_envs: bool = False, **kwargs): to match the new requirement described in lines 51-52 and demonstrated in line 303.
What this does
envs(envhub): add optional kwargs support to
make_envfor envhub