Add typed configurations to Arena environments

[cvolkcvolk]

Summary

Decouple environment construction from argparse

Detailed description

• Move every first-party environment's construction inputs from untyped argparse.Namespace reads into environment-specific dataclass configs.
• Make build(cfg) the typed, frontend-independent construction boundary so Hydra and future dispatchers can compose environments without emulating CLI state.
• Keep get_env(args_cli) as a temporary compatibility adapter, preserving existing command-line workflows and behavior.

Follow up: Make ExampleEnvironmentBase() a core base class

[greptile-apps]

Greptile Summary

This PR decouples all 19 first-party Arena environments from argparse.Namespace by introducing a per-environment @dataclass config that inherits from the new ArenaEnvironmentCfg base, a typed build(cfg) method, and a thin get_env(args_cli) compatibility adapter that translates CLI defaults into the typed config before delegating to build.

• New ArenaEnvironmentCfg base and ExampleEnvironmentBase[CfgT] generic enable type-safe composition; build() is intentionally non-abstract while get_env remains abstract, staging the migration without breaking existing callers.
• Compatibility layer is consistent and tested: each get_env adapter is covered by test_every_registered_legacy_adapter_translates_cli_defaults_to_its_typed_cfg, which patches build() and compares the captured cfg against freshly-parsed CLI defaults without instantiating Isaac Sim.
• Edge cases are handled: mutable list defaults use field(default_factory=...), the auto sentinel in GalileoG1LocomanipPickAndPlaceEnvironmentCfg preserves the legacy not hasattr(args_cli, \"auto\") semantics, and Maple's teleop_device is intentionally kept as a CLI-only option with a dedicated test and inline comment.

Confidence Score: 5/5

Safe to merge — all 19 environments apply the same adapter pattern consistently, the compatibility boundary is well-tested, and no existing behaviour changes.

The change is a pure structural refactoring: runtime paths through get_env are identical to before (same assets, same conditions), mutable default fields use factory functions, the mimic/auto sentinel logic is semantically equivalent to the old hasattr checks, and a dedicated test suite validates every adapter without requiring Isaac Sim. No data loss, no broken contracts, no unguarded type changes.

No files require special attention. The test file's use of object.new to skip init is intentional and safe since build is monkeypatched before any asset registry access.

Important Files Changed

Filename	Overview
isaaclab_arena/environments/arena_environment_cfg.py	New base dataclass for all environment configurations; correctly decorated with @DataClass so subclass machinery works properly.
isaaclab_arena_environments/example_environment_base.py	Adds Generic[ArenaEnvironmentCfgT] and a non-abstract build() stub; intentionally staged — get_env stays abstract while build() raises NotImplementedError until all environments migrate.
isaaclab_arena/tests/test_environment_cfgs.py	New test suite verifying the CLI-to-typed-cfg compatibility boundary; uses object.new to skip init safely since build is monkeypatched before any asset registry access.
isaaclab_arena_environments/galileo_g1_locomanip_pick_and_place_environment.py	Correctly translates the three-way hasattr check for mimic/auto into cfg.auto is None sentinel; behavior is equivalent for all existing CLI invocations.
isaaclab_arena_environments/pick_and_place_maple_table_environment.py	teleop_device correctly kept as CLI-only (not forwarded to cfg); comment and dedicated test both document and enforce this intent.
isaaclab_arena_environments/gr1_table_multi_object_no_collision_environment.py	num_envs and enable_cameras correctly promoted to cfg fields with getattr fallbacks in get_env to handle the shared global parser namespace.
isaaclab_arena_environments/galileo_g1_static_pick_and_place_environment.py	Adds typed cfg with lock_waist field; correctly uses module-level constants for default object and destination names.
isaaclab_arena_environments/sorting_environment.py	list fields correctly use field(default_factory=lambda: [...]) for mutable defaults; no shared-mutable-default risk.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant CLI as CLI / argparse
    participant Adapter as get_env(args_cli)
    participant Cfg as EnvironmentCfg (dataclass)
    participant Build as build(cfg)
    participant Env as IsaacLabArenaEnvironment

    CLI->>Adapter: Namespace with raw strings/bools
    Adapter->>Cfg: construct typed dataclass
    Cfg-->>Adapter: validated, defaulted cfg
    Adapter->>Build: build(cfg)
    Build->>Env: assemble scene, embodiment, task
    Env-->>CLI: IsaacLabArenaEnvironment

    note over Adapter,Cfg: Legacy compatibility boundary (get_env stays abstract)
    note over Build,Env: Typed Hydra-composable boundary (build raises NotImplementedError until migrated)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant CLI as CLI / argparse
    participant Adapter as get_env(args_cli)
    participant Cfg as EnvironmentCfg (dataclass)
    participant Build as build(cfg)
    participant Env as IsaacLabArenaEnvironment

    CLI->>Adapter: Namespace with raw strings/bools
    Adapter->>Cfg: construct typed dataclass
    Cfg-->>Adapter: validated, defaulted cfg
    Adapter->>Build: build(cfg)
    Build->>Env: assemble scene, embodiment, task
    Env-->>CLI: IsaacLabArenaEnvironment

    note over Adapter,Cfg: Legacy compatibility boundary (get_env stays abstract)
    note over Build,Env: Typed Hydra-composable boundary (build raises NotImplementedError until migrated)

Loading

_{Reviews (2): Last reviewed commit: "Consolidate environment config tests" | Re-trigger Greptile}

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

This PR introduces a per-environment typed ...Cfg dataclass for all 20 first-party environments and splits construction into a typed build(cfg) boundary plus a thin get_env(args_cli) compatibility adapter that maps the argparse namespace onto the config. It's a clean, careful, mechanical refactor: I verified that every dataclass field default matches its historical argparse default (spot-checked cube-goal-pose, static-G1, open-microwave, sorting, place-upright, put-and-close-door), and that the locomanip auto/mimic gate is faithfully preserved — --auto is a store_true (default False) in annotate_demos.py, so cfg.auto is None reproduces the original not hasattr(args_cli, "auto") exactly.

Design, Boundaries & Scope

The one thing worth weighing: the ~20 get_env overrides are now near-identical mechanical copies from args_cli into each Cfg. Since the new test already demonstrates this mapping is fully mechanical, a single generic get_env on the base (resolving the config type from __orig_bases__ and populating fields via getattr(args_cli, name, default)) could collapse all of them and let subclasses implement only build() + add_cli_args(). Left an inline note — not a blocker, and the adapters are clearly transitional, but since get_env is slated for deprecation a generic form would shrink the migration surface rather than grow it.

Findings

🟡 Warning: example_environment_base.py — ~20 duplicated get_env adapters could be a single generic implementation on the base class (see inline).

Test Coverage

test_environment_cfgs.py covers the adapter boundary well and correctly stays in Phase 1: it monkeypatches build and inspects the config produced by get_env, so no SimulationApp is needed and the inner/outer pattern doesn't apply. The build(cfg) path itself is left to the all-environments smoke test, which is reasonable. One gap the test doesn't cover: it can't catch a divergence between a dataclass-field default and its argparse default (it always passes explicit arg values), so I checked those defaults by hand — all consistent.

Verdict

Ship it — one optional design simplification worth considering.

[alexmillane]

LGTM. Configuring this way makes a lot more sense.

My only substantial comment is whether we could generate the argparse from the config class difinition? That would eliminate one (loose) contract for during the transition.