Skip to content

Fix the type annotations in Scene. #613

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.

Sign up for GitHub

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

Open
alexmillane wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix the type annotations in Scene. #613

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcb3270
Fix the type annotations in Scene.
alexmillane Apr 17, 2026
4149c75
Merge branch 'develop' into alex/cleanup/typing_in_scene
alexmillane Apr 21, 2026
e3ad6fc
Merge branch 'main' into alex/cleanup/typing_in_scene
alexmillane Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions isaaclab_arena/scene/scene.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@

from isaaclab.assets import AssetBaseCfg, RigidObjectCfg
from isaaclab.assets.articulation.articulation_cfg import ArticulationCfg
from isaaclab.managers import EventTermCfg
from isaaclab.sensors.contact_sensor.contact_sensor_cfg import ContactSensorCfg
from pxr import Gf, Usd, UsdGeom

from isaaclab_arena.assets.asset import Asset
from isaaclab_arena.assets.object import Object
from isaaclab_arena.assets.object_base import ObjectType
from isaaclab_arena.assets.object_base import ObjectBase, ObjectType
from isaaclab_arena.assets.object_reference import ObjectReference
from isaaclab_arena.assets.object_set import RigidObjectSet
from isaaclab_arena.environments.isaaclab_arena_manager_based_env import IsaacLabArenaManagerBasedRLEnvCfg
from isaaclab_arena.utils.configclass import make_configclass
from isaaclab_arena.utils.phyx_utils import add_contact_report
Expand All @@ -25,8 +24,8 @@

class Scene:

Copy link
Copy Markdown
Contributor

@isaaclab-review-bot isaaclab-review-bot Bot Apr 17, 2026

Choose a reason for hiding this comment

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

👍 The old list[Asset, RigidObjectSet] was invalid Python (list takes a single type parameter). list[ObjectBase] is the correct fix — it properly covers Object, ObjectReference, and RigidObjectSet through the class hierarchy.

def __init__(self, assets: list[Asset, RigidObjectSet] | None = None):
self.assets: dict[str, Asset | RigidObjectSet] = {}
def __init__(self, assets: list[ObjectBase] | None = None):
self.assets: dict[str, ObjectBase] = {}
# We add these here so a user can override them if they want.
self.observation_cfg = None
self.events_cfg = None
Expand All @@ -37,14 +36,14 @@ def __init__(self, assets: list[Asset, RigidObjectSet] | None = None):
if assets is not None:
self.add_assets(assets)

def add_asset(self, asset: Asset | RigidObjectSet):
def add_asset(self, asset: ObjectBase):
"""Add an asset to the scene.

Args:
asset: An Asset instance or a dictionary of Assets. If a dictionary is provided,
the keys will be used as the names of the assets and the values will be the list of assets.
asset: An :class:`~isaaclab_arena.assets.object_base.ObjectBase` instance (e.g. ``Object``,
``ObjectReference``, or ``RigidObjectSet``).
"""
if not isinstance(asset, Asset | RigidObjectSet):
if not isinstance(asset, ObjectBase):
raise ValueError(f"Invalid asset type: {type(asset)}")

if asset.name is None:
Expand All @@ -53,7 +52,7 @@ def add_asset(self, asset: Asset | RigidObjectSet):
# if name already exists, overwrite
self.assets[asset.name] = asset

def add_assets(self, assets: list[Asset | RigidObjectSet]):
def add_assets(self, assets: list[ObjectBase]):
all_assets = set(assets)
for asset in assets:
if isinstance(asset, ObjectReference):
Expand Down Expand Up @@ -81,7 +80,7 @@ def get_observation_cfg(self) -> Any:

def get_events_cfg(self) -> Any:
# Combine the configs into a configclass.
fields: list[tuple[str, type, AssetCfg]] = []
fields: list[tuple[str, type, EventTermCfg]] = []
Copy link
Copy Markdown
Contributor

@isaaclab-review-bot isaaclab-review-bot Bot Apr 17, 2026

Choose a reason for hiding this comment

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

👍 Good catch — this method builds EventTermCfg fields (from asset.get_event_cfg()), not AssetCfg fields. The old AssetCfg annotation was misleading.

for asset in self.assets.values():
event_cfg_name, event_cfg = asset.get_event_cfg()
if event_cfg is not None:
Expand Down Expand Up @@ -148,7 +147,7 @@ def export_scene_to_usd(scene: Scene, output_path: pathlib.Path, root_prim_path:
flattened_layer.Export(output_path.as_posix())


def _create_prim_from_asset(stage: Usd.Stage, asset: Asset) -> None:
def _create_prim_from_asset(stage: Usd.Stage, asset: ObjectBase) -> None:
Copy link
Copy Markdown
Contributor

@isaaclab-review-bot isaaclab-review-bot Bot Apr 17, 2026

Choose a reason for hiding this comment

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

🔵 Minor suggestion: The signature now accepts ObjectBase, but the body immediately asserts isinstance(asset, Object) (line 162). Since this private function only works with Object instances (it accesses asset.usd_path and asset.scale), you could keep the parameter typed as Object for tighter type safety, or add a brief comment noting the intentional narrowing. Non-blocking either way.

"""Adds a prim to the stage for the given asset.

This is used internally by the scene.export_to_usd method.
Expand Down
Loading