Unverified Commit cdfa954f authored by Mayank Mittal's avatar Mayank Mittal Committed by GitHub

Fixes support for `classmethod` when defining a configclass (#901)

# Description

Previously, the configclass instance did not properly parse
classmethods. For instance, the following would fail:

```python
from __future__ import annotations

"""Launch Isaac Sim Simulator first."""

from omni.isaac.lab.app import AppLauncher

# launch omniverse app
app_launcher = AppLauncher(headless=True)

"""Rest everything follows."""

from omni.isaac.lab.utils.configclass import configclass


@configclass
class DummyClass:

    a: int = 5

    def instance_method(self):
        print("Value of a: ", self.a)

    @classmethod
    def class_method(cls, value: int) -> DummyClass:
        return cls(a=value)


cfg = DummyClass()

# check all methods are callable
cfg.instance_method()
new_cfg1 = cfg.class_method(20)

# create the same config instance using class method
new_cfg2 = DummyClass.class_method(20)
```

This MR fixes the checks to make sure class-methods remain bound to the
class and do not become instance variables.

## 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
`./isaaclab.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] 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
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
parent 8fcbf85f
[package] [package]
# Note: Semantic Versioning is used: https://semver.org/ # Note: Semantic Versioning is used: https://semver.org/
version = "0.22.3" version = "0.22.4"
# Description # Description
title = "Isaac Lab framework for Robot Learning" title = "Isaac Lab framework for Robot Learning"
......
Changelog Changelog
--------- ---------
0.22.4 (2024-08-29)
~~~~~~~~~~~~~~~~~~~
Fixed
^^^^^
* Fixed the support for class-bounded methods when creating a configclass
out of them. Earlier, these methods were being made as instance methods
which required initialization of the class to call the class-methods.
0.22.3 (2024-08-28) 0.22.3 (2024-08-28)
~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
"""Sub-module that provides a wrapper around the Python 3.7 onwards ``dataclasses`` module.""" """Sub-module that provides a wrapper around the Python 3.7 onwards ``dataclasses`` module."""
import inspect import inspect
import types
from collections.abc import Callable from collections.abc import Callable
from copy import deepcopy from copy import deepcopy
from dataclasses import MISSING, Field, dataclass, field, replace from dataclasses import MISSING, Field, dataclass, field, replace
...@@ -392,6 +393,11 @@ def _skippable_class_member(key: str, value: Any, hints: dict | None = None) -> ...@@ -392,6 +393,11 @@ def _skippable_class_member(key: str, value: Any, hints: dict | None = None) ->
return True return True
# skip functions bounded to class # skip functions bounded to class
if callable(value): if callable(value):
# FIXME: This doesn't yet work for static methods because they are essentially seen as function types.
# check for class methods
if isinstance(value, types.MethodType):
return True
# check for instance methods
signature = inspect.signature(value) signature = inspect.signature(value)
if "self" in signature.parameters or "cls" in signature.parameters: if "self" in signature.parameters or "cls" in signature.parameters:
return True return True
......
...@@ -249,7 +249,7 @@ class TestCamera(unittest.TestCase): ...@@ -249,7 +249,7 @@ class TestCamera(unittest.TestCase):
prim_path="/World/Camera_2", prim_path="/World/Camera_2",
update_period=0, update_period=0,
data_types=["distance_to_image_plane"], data_types=["distance_to_image_plane"],
spawn=sim_utils.PinholeCameraCfg().from_intrinsic_matrix( spawn=sim_utils.PinholeCameraCfg.from_intrinsic_matrix(
intrinsic_matrix=intrinsic_matrix, intrinsic_matrix=intrinsic_matrix,
width=128, width=128,
height=128, height=128,
......
...@@ -221,7 +221,7 @@ class TestWarpCamera(unittest.TestCase): ...@@ -221,7 +221,7 @@ class TestWarpCamera(unittest.TestCase):
update_period=0, update_period=0,
offset=RayCasterCameraCfg.OffsetCfg(pos=(0.0, 0.0, 0.0), rot=(1.0, 0.0, 0.0, 0.0), convention="world"), offset=RayCasterCameraCfg.OffsetCfg(pos=(0.0, 0.0, 0.0), rot=(1.0, 0.0, 0.0, 0.0), convention="world"),
debug_vis=False, debug_vis=False,
pattern_cfg=patterns.PinholeCameraPatternCfg().from_intrinsic_matrix( pattern_cfg=patterns.PinholeCameraPatternCfg.from_intrinsic_matrix(
intrinsic_matrix=intrinsic_matrix, intrinsic_matrix=intrinsic_matrix,
height=self.camera_cfg.pattern_cfg.height, height=self.camera_cfg.pattern_cfg.height,
width=self.camera_cfg.pattern_cfg.width, width=self.camera_cfg.pattern_cfg.width,
...@@ -665,7 +665,7 @@ class TestWarpCamera(unittest.TestCase): ...@@ -665,7 +665,7 @@ class TestWarpCamera(unittest.TestCase):
mesh_prim_paths=["/World/defaultGroundPlane"], mesh_prim_paths=["/World/defaultGroundPlane"],
offset=RayCasterCameraCfg.OffsetCfg(pos=offset_pos, rot=offset_rot, convention="ros"), offset=RayCasterCameraCfg.OffsetCfg(pos=offset_pos, rot=offset_rot, convention="ros"),
debug_vis=False, debug_vis=False,
pattern_cfg=patterns.PinholeCameraPatternCfg().from_intrinsic_matrix( pattern_cfg=patterns.PinholeCameraPatternCfg.from_intrinsic_matrix(
intrinsic_matrix=intrinsics, intrinsic_matrix=intrinsics,
height=540, height=540,
width=960, width=960,
...@@ -677,7 +677,7 @@ class TestWarpCamera(unittest.TestCase): ...@@ -677,7 +677,7 @@ class TestWarpCamera(unittest.TestCase):
camera_usd_cfg = CameraCfg( camera_usd_cfg = CameraCfg(
prim_path="/World/Camera_usd", prim_path="/World/Camera_usd",
offset=CameraCfg.OffsetCfg(pos=offset_pos, rot=offset_rot, convention="ros"), offset=CameraCfg.OffsetCfg(pos=offset_pos, rot=offset_rot, convention="ros"),
spawn=PinholeCameraCfg().from_intrinsic_matrix( spawn=PinholeCameraCfg.from_intrinsic_matrix(
intrinsic_matrix=intrinsics, intrinsic_matrix=intrinsics,
height=540, height=540,
width=960, width=960,
......
...@@ -315,7 +315,7 @@ class TestTiledCamera(unittest.TestCase): ...@@ -315,7 +315,7 @@ class TestTiledCamera(unittest.TestCase):
camera_tiled_cfg = TiledCameraCfg( camera_tiled_cfg = TiledCameraCfg(
prim_path="/World/Camera_tiled", prim_path="/World/Camera_tiled",
offset=TiledCameraCfg.OffsetCfg(pos=offset_pos, rot=offset_rot, convention="ros"), offset=TiledCameraCfg.OffsetCfg(pos=offset_pos, rot=offset_rot, convention="ros"),
spawn=sim_utils.PinholeCameraCfg().from_intrinsic_matrix( spawn=sim_utils.PinholeCameraCfg.from_intrinsic_matrix(
intrinsic_matrix=intrinsics, intrinsic_matrix=intrinsics,
height=540, height=540,
width=960, width=960,
...@@ -329,7 +329,7 @@ class TestTiledCamera(unittest.TestCase): ...@@ -329,7 +329,7 @@ class TestTiledCamera(unittest.TestCase):
camera_usd_cfg = CameraCfg( camera_usd_cfg = CameraCfg(
prim_path="/World/Camera_usd", prim_path="/World/Camera_usd",
offset=CameraCfg.OffsetCfg(pos=offset_pos, rot=offset_rot, convention="ros"), offset=CameraCfg.OffsetCfg(pos=offset_pos, rot=offset_rot, convention="ros"),
spawn=sim_utils.PinholeCameraCfg().from_intrinsic_matrix( spawn=sim_utils.PinholeCameraCfg.from_intrinsic_matrix(
intrinsic_matrix=intrinsics, intrinsic_matrix=intrinsics,
height=540, height=540,
width=960, width=960,
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
# #
# SPDX-License-Identifier: BSD-3-Clause # SPDX-License-Identifier: BSD-3-Clause
from __future__ import annotations
# NOTE: While we don't actually use the simulation app in this test, we still need to launch it # NOTE: While we don't actually use the simulation app in this test, we still need to launch it
# because warp is only available in the context of a running simulation # because warp is only available in the context of a running simulation
"""Launch Isaac Sim Simulator first.""" """Launch Isaac Sim Simulator first."""
...@@ -292,6 +294,20 @@ class FunctionImplementedDemoCfg: ...@@ -292,6 +294,20 @@ class FunctionImplementedDemoCfg:
self.a = a self.a = a
@configclass
class ClassFunctionImplementedDemoCfg:
"""Dummy configuration class with function members defined in the class."""
a: int = 5
def instance_method(self):
print("Value of a: ", self.a)
@classmethod
def class_method(cls, value: int) -> ClassFunctionImplementedDemoCfg:
return cls(a=value)
""" """
Dummy configuration: Nested dictionaries Dummy configuration: Nested dictionaries
""" """
...@@ -618,12 +634,31 @@ class TestConfigClass(unittest.TestCase): ...@@ -618,12 +634,31 @@ class TestConfigClass(unittest.TestCase):
self.assertEqual(cfg.func_in_dict["func"](), 1) self.assertEqual(cfg.func_in_dict["func"](), 1)
def test_function_impl_config(self): def test_function_impl_config(self):
"""Tests having function defined in the class instance."""
cfg = FunctionImplementedDemoCfg() cfg = FunctionImplementedDemoCfg()
# change value # change value
self.assertEqual(cfg.a, 5) self.assertEqual(cfg.a, 5)
cfg.set_a(10) cfg.set_a(10)
self.assertEqual(cfg.a, 10) self.assertEqual(cfg.a, 10)
def test_class_function_impl_config(self):
"""Tests having class and static function defined in the class instance."""
cfg = ClassFunctionImplementedDemoCfg()
# check that the annotations are correct
self.assertDictEqual(cfg.__annotations__, {"a": "int"})
# check all methods are callable
cfg.instance_method()
new_cfg1 = cfg.class_method(20)
# check value is correct
self.assertEqual(new_cfg1.a, 20)
# create the same config instance using class method
new_cfg2 = ClassFunctionImplementedDemoCfg.class_method(20)
# check value is correct
self.assertEqual(new_cfg2.a, 20)
def test_dict_conversion_functions_config(self): def test_dict_conversion_functions_config(self):
"""Tests conversion of config with functions into dictionary.""" """Tests conversion of config with functions into dictionary."""
cfg = FunctionsDemoCfg() cfg = FunctionsDemoCfg()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment