Skip to content

refactor(env): introduce explicit gym ID handling in EnvConfig/factory - #1008

Closed
bsprenger wants to merge 3 commits into
huggingface:mainfrom
bsprenger:feat/gym-factory
Closed

bsprenger wants to merge 3 commits into
huggingface:mainfrom
bsprenger:feat/gym-factory

Conversation

@bsprenger

@bsprenger bsprenger commented Apr 21, 2025 •

Copy link
Copy Markdown
Contributor

What this does (✨ Feature)

This commit introduces properties for the gym package/ID associated with an environment config. They default to the current defaults (gym_{package_name}/{task_id}) to avoid breaking changes, but allow for easier use of external gym environments.

Subclasses of EnvConfig can override the default properties to allow the factory to import (i.e. register) the gym env from a specific module, and also instantiate the env from any ID string.

How it was tested

  • Tested to make sure existing environment configs still work with this change (ran test suite)
  • Added a test for a dummy external env config in (tests/envs/test_envs.py::test_factory_custom_gym_id)

How to checkout & try? (for the reviewer)

pytest -sx tests/envs/test_envs.py

@bsprenger
bsprenger force-pushed the feat/gym-factory branch 2 times, most recently from 1b98e80 to e5c431d Compare April 21, 2025 12:15
@bsprenger

Copy link
Copy Markdown
Contributor Author

Closely related to #807 (merged). This is a next step to help use LeRobot with external projects 😊

@imstevenpmwork I'm not sure who to tag who could review this, let me know if you think I should tag someone in particular!

@imstevenpmwork imstevenpmwork added simulation Matters involving system simulation or modeling refactor labels Jun 6, 2025
@aliberts
aliberts requested a review from AdilZouitine June 10, 2025 16:52
AdilZouitine
AdilZouitine previously approved these changes Jun 15, 2025

@AdilZouitine AdilZouitine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@bsprenger
bsprenger force-pushed the feat/gym-factory branch 2 times, most recently from bc58fb7 to e69070a Compare July 22, 2025 13:48
@bsprenger

Copy link
Copy Markdown
Contributor Author

Hi @AdilZouitine , thanks for the approval!

I had to rebase this commit due to the common->src folder structure change. Otherwise, the code is exactly the same as when you approved it.

Can you take a look again? What needs to happen to get this merged?

@bsprenger

Copy link
Copy Markdown
Contributor Author

Hi @AdilZouitine, any update on this? What can I do to help get this merged?

This commit introduces properties for the gym package/ID associated
with and environment config. They default to the current defaults
(`gym_{package_name}/{task_id}`) to avoid breaking changes, but allow
for easier use of external gym environments.

Subclasses of `EnvConfig` can override the default properties to allow
the factory to import (i.e. register) the gym env from a specific module,
and also instantiate the env from any ID string.
Copilot AI review requested due to automatic review settings September 29, 2025 05:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces explicit gym ID handling in EnvConfig by adding package_name and gym_id properties to support external gym environments. The changes maintain backward compatibility with existing environment configs while enabling subclasses to override these properties for custom gym environments.

  • Adds package_name and gym_id properties to EnvConfig with default implementations
  • Updates the factory function to use the new properties instead of hardcoded string formatting
  • Improves error handling and messaging when environments are not found in the gym registry

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/lerobot/envs/configs.py Adds package_name and gym_id properties to EnvConfig base class
src/lerobot/envs/factory.py Updates make_env to use new properties and improves error handling
tests/envs/test_envs.py Adds test for custom gym ID functionality with dummy environment

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread tests/envs/test_envs.py Resolved
@CarolinePascal

Copy link
Copy Markdown
Collaborator

Hi !

I'm closing this PR as it was superseded by #2234 (now merged).

Best,

Caroline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

simulation Matters involving system simulation or modeling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants