Changes all attribute names for classes in configclass to `class_type` (#161)

# Description

Currently, the code has a mix of `cls_name` and `cls` in the
configuration classes. Since `cls` is a reserved key from Python, we
should avoid using this name for attributes. This MR changes all the
occurrences to become `class_type` instead.

Fixes #141 

## Type of change

- Bug fix (non-breaking change which fixes an issue)

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./orbit.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file

---------
Co-authored-by: Mayank Mittal <mittalma@leggedrobotics.com>

docs/source/refs/contributing.rst

@@ -67,6 +67,29 @@ For documentation, we adopt the `Google Style Guide <https://sphinxcontrib-napol
 for docstrings. We use `Sphinx <https://www.sphinx-doc.org/en/master/>`__ for generating the documentation.
 Please make sure that your code is well-documented and follows the guidelines.
 
+Circular Imports
+^^^^^^^^^^^^^^^^
+
+Circular imports happen when two modules import each other, which is a common issue in Python.
+You can prevent circular imports by adhering to the best practices outlined in this
+`StackOverflow post <https://stackoverflow.com/questions/744373/circular-or-cyclic-imports-in-python>`__.
+
+In general, it is essential to avoid circular imports as they can lead to unpredictable behavior.
+
+However, in our codebase, we encounter circular imports at a sub-package level. This situation arises
+due to our specific code structure. We organize classes or functions and their corresponding configuration
+objects into separate files. This separation enhances code readability and maintainability. Nevertheless,
+it can result in circular imports because, in many configuration objects, we specify classes or functions
+as default values using the attributes ``class_type`` and ``func`` respectively.
+
+To address circular imports, we leverage the `typing.TYPE_CHECKING
+<https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING>`_ variable. This special variable is
+evaluated only during type-checking, allowing us to import classes or functions in the configuration objects
+without triggering circular imports.
+
+It is important to note that this is the sole instance within our codebase where circular imports are used
+and are acceptable. In all other scenarios, we adhere to best practices and recommend that you do the same.
+
 Type-hinting
 ^^^^^^^^^^^^
 

source/extensions/omni.isaac.orbit/config/extension.toml

 [package]
 
 # Note: Semantic Versioning is used: https://semver.org/
-version = "0.9.5"
+version = "0.9.6"
 
 # Description
 title = "ORBIT framework for Robot Learning"

source/extensions/omni.isaac.orbit/docs/CHANGELOG.rst

 Changelog
 ---------
 
+0.9.6 (2023-09-26)
+~~~~~~~~~~~~~~~~~~
+
+Fixed
+^^^^^
+
+* Changed class-level configuration classes to refer to class types using ``class_type`` attribute instead
+  of ``cls`` or ``cls_name``.
+
+
 0.9.5 (2023-09-25)
 ~~~~~~~~~~~~~~~~~~
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/actuators/actuator_cfg.py

@@ -18,8 +18,11 @@ from .actuator_base import ActuatorBase
 class ActuatorBaseCfg:
     """Configuration for default actuators in an articulation."""
 
-    cls: type[ActuatorBase] = MISSING
-    """Actuator class."""
+    class_type: type[ActuatorBase] = MISSING
+    """The associated actuator class.
+
+    The class should inherit from :class:`omni.isaac.orbit.actuators.actuator_base.ActuatorBase`.
+    """
 
     joint_names_expr: list[str] = MISSING
     """Articulation's joint names that are part of the group.
@@ -66,7 +69,7 @@ class ImplicitActuatorCfg(ActuatorBaseCfg):
         The PD control is handled implicitly by the simulation.
     """
 
-    cls = actuator_pd.ImplicitActuator
+    class_type: type = actuator_pd.ImplicitActuator
 
 
 """
@@ -78,14 +81,14 @@ Explicit Actuator Models.
 class IdealPDActuatorCfg(ActuatorBaseCfg):
     """Configuration for an ideal PD actuator."""
 
-    cls = actuator_pd.IdealPDActuator
+    class_type: type = actuator_pd.IdealPDActuator
 
 
 @configclass
 class DCMotorCfg(IdealPDActuatorCfg):
     """Configuration for direct control (DC) motor actuator model."""
 
-    cls = actuator_pd.DCMotor
+    class_type: type = actuator_pd.DCMotor
 
     saturation_effort: float = MISSING
     """Peak motor force/torque of the electric DC motor (in N-m)."""
@@ -95,7 +98,7 @@ class DCMotorCfg(IdealPDActuatorCfg):
 class ActuatorNetLSTMCfg(DCMotorCfg):
     """Configuration for LSTM-based actuator model."""
 
-    cls = actuator_net.ActuatorNetLSTM
+    class_type: type = actuator_net.ActuatorNetLSTM
     # we don't use stiffness and damping for actuator net
     stiffness = None
     damping = None
@@ -108,7 +111,7 @@ class ActuatorNetLSTMCfg(DCMotorCfg):
 class ActuatorNetMLPCfg(DCMotorCfg):
     """Configuration for MLP-based actuator model."""
 
-    cls = actuator_net.ActuatorNetMLP
+    class_type: type = actuator_net.ActuatorNetMLP
     # we don't use stiffness and damping for actuator net
     stiffness = None
     damping = None

source/extensions/omni.isaac.orbit/omni/isaac/orbit/assets/articulation/articulation.py

@@ -492,8 +492,7 @@ class Articulation(RigidObject):
             if len(joint_names) == self.num_joints:
                 joint_ids = ...
             # create actuator collection
-            actuator_cls = actuator_cfg.cls
-            actuator: ActuatorBase = actuator_cls(
+            actuator: ActuatorBase = actuator_cfg.class_type(
                 cfg=actuator_cfg,
                 joint_names=joint_names,
                 joint_ids=joint_ids,
@@ -502,7 +501,7 @@ class Articulation(RigidObject):
             )
             # log information on actuator groups
             carb.log_info(
-                f"Actuator collection: {actuator_name} with model '{actuator_cls.__name__}' and "
+                f"Actuator collection: {actuator_name} with model '{actuator_cfg.class_type.__name__}' and "
                 f"joint names: {joint_names} [{joint_ids}]."
             )
             # store actuator group

source/extensions/omni.isaac.orbit/omni/isaac/orbit/assets/articulation/articulation_cfg.py

@@ -17,7 +17,7 @@ from .articulation import Articulation
 class ArticulationCfg(RigidObjectCfg):
     """Configuration parameters for an articulation."""
 
-    cls_name = Articulation
+    class_type: type = Articulation
 
     @configclass
     class InitialStateCfg(RigidObjectCfg.InitialStateCfg):

source/extensions/omni.isaac.orbit/omni/isaac/orbit/assets/asset_base.py

@@ -7,13 +7,14 @@ from __future__ import annotations
 
 import re
 from abc import ABC, abstractmethod
-from typing import Any, Sequence
+from typing import TYPE_CHECKING, Any, Sequence
 
 import omni.isaac.core.utils.prims as prim_utils
 import omni.kit.app
 import omni.physx
 
-from .asset_base_cfg import AssetBaseCfg
+if TYPE_CHECKING:
+    from .asset_base_cfg import AssetBaseCfg
 
 
 class AssetBase(ABC):

source/extensions/omni.isaac.orbit/omni/isaac/orbit/assets/asset_base_cfg.py

@@ -6,12 +6,13 @@
 from __future__ import annotations
 
 from dataclasses import MISSING
-from typing import ClassVar
 from typing_extensions import Literal
 
 from omni.isaac.orbit.sim import SpawnerCfg
 from omni.isaac.orbit.utils import configclass
 
+from .asset_base import AssetBase
+
 
 @configclass
 class AssetBaseCfg:
@@ -29,8 +30,11 @@ class AssetBaseCfg:
         Defaults to (1.0, 0.0, 0.0, 0.0).
         """
 
-    cls_name: ClassVar[str] = MISSING
-    """Class name of the asset."""
+    class_type: type[AssetBase] = MISSING
+    """The associated asset class.
+
+    The class should inherit from :class:`omni.isaac.orbit.assets.asset_base.AssetBase`.
+    """
 
     prim_path: str = MISSING
     """Prim path (or expression) to the asset.

source/extensions/omni.isaac.orbit/omni/isaac/orbit/assets/rigid_object/rigid_object_cfg.py

@@ -15,7 +15,7 @@ from .rigid_object import RigidObject
 class RigidObjectCfg(AssetBaseCfg):
     """Configuration parameters for a rigid object."""
 
-    cls_name = RigidObject
+    class_type: type = RigidObject
 
     @configclass
     class InitialStateCfg(AssetBaseCfg.InitialStateCfg):

source/extensions/omni.isaac.orbit/omni/isaac/orbit/command_generators/command_generator_cfg.py

@@ -22,8 +22,12 @@ Base command generator.
 class CommandGeneratorBaseCfg:
     """Configuration for the base command generator."""
 
-    class_name: type[CommandGeneratorBase] = MISSING
-    """The command generator class to use."""
+    class_type: type[CommandGeneratorBase] = MISSING
+    """The associated command generator class to use.
+
+    The class should inherit from :class:`omni.isaac.orbit.command_generators.command_generator_base.CommandGeneratorBase`.
+    """
+
     resampling_time_range: tuple[float, float] = MISSING
     """Time before commands are changed [s]."""
     debug_vis: bool = False
@@ -39,7 +43,7 @@ Locomotion-specific command generators.
 class UniformVelocityCommandGeneratorCfg(CommandGeneratorBaseCfg):
     """Configuration for the uniform velocity command generator."""
 
-    class_name = UniformVelocityCommandGenerator
+    class_type: type = UniformVelocityCommandGenerator
 
     asset_name: str = MISSING
     """Name of the asset in the environment for which the commands are generated."""
@@ -73,7 +77,7 @@ class UniformVelocityCommandGeneratorCfg(CommandGeneratorBaseCfg):
 class NormalVelocityCommandGeneratorCfg(UniformVelocityCommandGeneratorCfg):
     """Configuration for the normal velocity command generator."""
 
-    class_name = NormalVelocityCommandGenerator
+    class_type: type = NormalVelocityCommandGenerator
     heading_command: bool = False  # --> we don't use heading command for normal velocity command.
 
     @configclass
@@ -104,7 +108,7 @@ class NormalVelocityCommandGeneratorCfg(UniformVelocityCommandGeneratorCfg):
 class TerrainBasedPositionCommandGeneratorCfg(CommandGeneratorBaseCfg):
     """Configuration for the terrain-based position command generator."""
 
-    class_name = TerrainBasedPositionCommandGenerator
+    class_type: type = TerrainBasedPositionCommandGenerator
 
     asset_name: str = MISSING
     """Name of the asset in the environment for which the commands are generated."""

source/extensions/omni.isaac.orbit/omni/isaac/orbit/compat/actuators/group/actuator_group.py

@@ -93,7 +93,7 @@ class ActuatorGroup:
         # process configuration
         # -- create actuator model
         if self.model_type == "explicit":
-            actuator_model_cls = eval(self.cfg.model_cfg.cls_name)
+            actuator_model_cls = eval(self.cfg.model_cfg.class_type)
             self.model = actuator_model_cls(
                 cfg=self.cfg.model_cfg,
                 num_actuators=self.num_actuators,

source/extensions/omni.isaac.orbit/omni/isaac/orbit/compat/actuators/group/actuator_group_cfg.py

@@ -29,7 +29,7 @@ from .actuator_control_cfg import ActuatorControlCfg
 class ActuatorGroupCfg:
     """Configuration for default group of actuators in an articulation."""
 
-    cls_name: ClassVar[str] = "ActuatorGroup"
+    class_type: ClassVar[str] = "ActuatorGroup"
     """Name of the associated actuator group class. Used to construct the actuator group."""
 
     dof_names: list[str] = MISSING
@@ -46,7 +46,7 @@ class ActuatorGroupCfg:
 class GripperActuatorGroupCfg(ActuatorGroupCfg):
     """Configuration for mimicking actuators in a gripper."""
 
-    cls_name: ClassVar[str] = "GripperActuatorGroup"
+    class_type: ClassVar[str] = "GripperActuatorGroup"
 
     speed: float = MISSING
     """The speed at which gripper should close. (used with velocity command type.)"""
@@ -74,4 +74,4 @@ class GripperActuatorGroupCfg(ActuatorGroupCfg):
 class NonHolonomicKinematicsGroupCfg(ActuatorGroupCfg):
     """Configuration for formulating non-holonomic base constraint."""
 
-    cls_name: ClassVar[str] = "NonHolonomicKinematicsGroup"
+    class_type: ClassVar[str] = "NonHolonomicKinematicsGroup"

source/extensions/omni.isaac.orbit/omni/isaac/orbit/envs/base_env.py

@@ -147,7 +147,7 @@ class BaseEnv:
         # prepare the managers
         # note: this order is important since observation manager needs to know the command and action managers
         # -- command manager
-        self.command_manager: CommandGeneratorBase = self.cfg.commands.class_name(self.cfg.commands, self)
+        self.command_manager: CommandGeneratorBase = self.cfg.commands.class_type(self.cfg.commands, self)
         print("[INFO] Command Manager: ", self.command_manager)
         # -- action manager
         self.action_manager = ActionManager(self.cfg.actions, self)

source/extensions/omni.isaac.orbit/omni/isaac/orbit/envs/mdp/actions/actions_cfg.py

@@ -39,7 +39,7 @@ class JointPositionActionCfg(JointActionCfg):
     See :class:`JointPositionAction` for more details.
     """
 
-    cls: type[ActionTerm] = joint_actions.JointPositionAction
+    class_type: type[ActionTerm] = joint_actions.JointPositionAction
 
     use_default_offset: bool = True
     """Whether to use default joint positions configured in the articulation asset as offset.
@@ -56,7 +56,7 @@ class JointVelocityActionCfg(JointActionCfg):
     See :class:`JointVelocityAction` for more details.
     """
 
-    cls: type[ActionTerm] = joint_actions.JointVelocityAction
+    class_type: type[ActionTerm] = joint_actions.JointVelocityAction
 
     use_default_offset: bool = True
     """Whether to use default joint velocities configured in the articulation asset as offset.
@@ -73,7 +73,7 @@ class JointEffortActionCfg(JointActionCfg):
     See :class:`JointEffortAction` for more details.
     """
 
-    cls: type[ActionTerm] = joint_actions.JointEffortAction
+    class_type: type[ActionTerm] = joint_actions.JointEffortAction
 
 
 ##
@@ -103,7 +103,7 @@ class BinaryJointPositionActionCfg(BinaryJointActionCfg):
     See :class:`BinaryJointPositionAction` for more details.
     """
 
-    cls: type[ActionTerm] = binary_joint_actions.BinaryJointPositionAction
+    class_type: type[ActionTerm] = binary_joint_actions.BinaryJointPositionAction
 
 
 @configclass
@@ -113,7 +113,7 @@ class BinaryJointVelocityActionCfg(BinaryJointActionCfg):
     See :class:`BinaryJointVelocityAction` for more details.
     """
 
-    cls: type[ActionTerm] = binary_joint_actions.BinaryJointVelocityAction
+    class_type: type[ActionTerm] = binary_joint_actions.BinaryJointVelocityAction
 
 
 ##
@@ -128,7 +128,7 @@ class NonHolonomicActionCfg(ActionTermCfg):
     See :class:`NonHolonomicAction` for more details.
     """
 
-    cls: type[ActionTerm] = non_holonomic_actions.NonHolonomicAction
+    class_type: type[ActionTerm] = non_holonomic_actions.NonHolonomicAction
 
     body_name: str = MISSING
     """Name of the body which has the dummy mechanism connected to."""

source/extensions/omni.isaac.orbit/omni/isaac/orbit/managers/action_manager.py

@@ -257,7 +257,7 @@ class ActionManager(ManagerBase):
                     f"Configuration for the term '{term_name}' is not of type ActionTermCfg. Received '{type(term_cfg)}'."
                 )
             # create the action term
-            term = term_cfg.cls(term_cfg, self._env)
+            term = term_cfg.class_type(term_cfg, self._env)
             # sanity check if term is valid type
             if not isinstance(term, ActionTerm):
                 raise TypeError(f"Returned object for the term '{term_name}' is not of type ActionType.")

source/extensions/omni.isaac.orbit/omni/isaac/orbit/managers/manager_cfg.py

@@ -103,11 +103,18 @@ class ManagerBaseTermCfg:
 class ActionTermCfg:
     """Configuration for an action term."""
 
-    cls: type[ActionTerm] = MISSING
-    """Class of the action term."""
+    class_type: type[ActionTerm] = MISSING
+    """The associated action term class.
+
+    The class should inherit from :class:`omni.isaac.orbit.managers.action_manager.ActionTerm`.
+    """
 
     asset_name: str = MISSING
-    """Name of the asset (object or robot) on which action is applied."""
+    """The name of the scene entity.
+
+    This is the name defined in the scene configuration file. See the :class:`InteractiveSceneCfg`
+    class for more details.
+    """
 
 
 ##

source/extensions/omni.isaac.orbit/omni/isaac/orbit/scene/interactive_scene.py

@@ -336,13 +336,13 @@ class InteractiveScene:
                 # terrains are special entities since they define environment origins
                 asset_cfg.num_envs = self.cfg.num_envs
                 asset_cfg.env_spacing = self.cfg.env_spacing
-                self.terrain = asset_cfg.cls_name(asset_cfg)
+                self.terrain = asset_cfg.class_type(asset_cfg)
             elif isinstance(asset_cfg, ArticulationCfg):
-                self.articulations[asset_name] = asset_cfg.cls_name(asset_cfg)
+                self.articulations[asset_name] = asset_cfg.class_type(asset_cfg)
             elif isinstance(asset_cfg, RigidObjectCfg):
-                self.rigid_objects[asset_name] = asset_cfg.cls_name(asset_cfg)
+                self.rigid_objects[asset_name] = asset_cfg.class_type(asset_cfg)
             elif isinstance(asset_cfg, SensorBaseCfg):
-                self.sensors[asset_name] = asset_cfg.cls_name(asset_cfg)
+                self.sensors[asset_name] = asset_cfg.class_type(asset_cfg)
             elif isinstance(asset_cfg, AssetBaseCfg):
                 # manually spawn asset
                 if asset_cfg.spawn is not None:

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sensors/camera/camera_cfg.py

@@ -38,7 +38,7 @@ class CameraCfg(SensorBaseCfg):
 
         """
 
-    cls_name = Camera
+    class_type: type = Camera
 
     offset: OffsetCfg = OffsetCfg()
     """The offset pose of the sensor's frame from the sensor's parent frame. Defaults to identity.

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sensors/contact_sensor/contact_sensor_cfg.py

@@ -16,7 +16,7 @@ from .contact_sensor import ContactSensor
 class ContactSensorCfg(SensorBaseCfg):
     """Configuration for the contact sensor."""
 
-    cls_name = ContactSensor
+    class_type: type = ContactSensor
 
     filter_prim_paths_expr: list[str] = list()
     """The list of primitive paths to filter contacts with.

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sensors/ray_caster/ray_caster_cfg.py

@@ -29,7 +29,7 @@ class RayCasterCfg(SensorBaseCfg):
         rot: tuple[float, float, float, float] = (1.0, 0.0, 0.0, 0.0)
         """Quaternion rotation ``(w, x, y, z)`` w.r.t. the parent frame. Defaults to (1.0, 0.0, 0.0, 0.0)."""
 
-    cls_name = RayCaster
+    class_type: type = RayCaster
 
     mesh_prim_paths: list[str] = MISSING
     """The list of mesh primitive paths to ray cast against.

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sensors/sensor_base.py

@@ -13,14 +13,15 @@ from __future__ import annotations
 
 import torch
 from abc import ABC, abstractmethod
-from typing import Any, Sequence
+from typing import TYPE_CHECKING, Any, Sequence
 
 import omni.isaac.core.utils.prims as prim_utils
 import omni.kit.app
 import omni.physx
 from omni.isaac.core.simulation_context import SimulationContext
 
-from .sensor_base_cfg import SensorBaseCfg
+if TYPE_CHECKING:
+    from .sensor_base_cfg import SensorBaseCfg
 
 
 class SensorBase(ABC):

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sensors/sensor_base_cfg.py

@@ -6,17 +6,21 @@
 from __future__ import annotations
 
 from dataclasses import MISSING
-from typing import ClassVar
 
 from omni.isaac.orbit.utils import configclass
 
+from .sensor_base import SensorBase
+
 
 @configclass
 class SensorBaseCfg:
     """Configuration parameters for a sensor."""
 
-    cls_name: ClassVar[type] = MISSING
-    """The associated sensor class."""
+    class_type: type[SensorBase] = MISSING
+    """The associated sensor class.
+
+    The class should inherit from :class:`omni.isaac.orbit.sensors.sensor_base.SensorBase`.
+    """
 
     prim_path: str = MISSING
     """Prim path (or expression) to the asset.

source/extensions/omni.isaac.orbit/omni/isaac/orbit/terrains/terrain_importer_cfg.py

@@ -22,8 +22,11 @@ if TYPE_CHECKING:
 class TerrainImporterCfg:
     """Configuration for the terrain manager."""
 
-    cls_name: type = TerrainImporter
-    """The class name of the terrain importer."""
+    class_type: type = TerrainImporter
+    """The class to use for the terrain importer.
+
+    Defaults to :class:`omni.isaac.orbit.terrains.terrain_importer.TerrainImporter`.
+    """
 
     collision_group: int = -1
     """The collision group of the terrain. Defaults to -1."""

source/extensions/omni.isaac.orbit/test/utils/test_configclass.py

@@ -231,7 +231,7 @@ class OutsideClassCfg:
 
             u: list[int] = [1, 2, 3]
 
-        class_name: type = DummyClass
+        class_type: type = DummyClass
         b: str = "dummy"
 
     inside: InsideClassCfg = InsideClassCfg()
@@ -638,7 +638,7 @@ class TestConfigClass(unittest.TestCase):
         self.assertNotIn("InsideInsideClassCfg", OutsideClassCfg.InsideClassCfg.__annotations__)
         self.assertNotIn("InsideInsideClassCfg", cfg.inside.__annotations__)
         # check values
-        self.assertEqual(cfg.inside.class_name, DummyClass)
+        self.assertEqual(cfg.inside.class_type, DummyClass)
         self.assertEqual(cfg.inside.b, "dummy_changed")
         self.assertEqual(cfg.x, 20)