From 7325f65cef061ff4ccbfd6ea3c4b5341eb0ad2ae Mon Sep 17 00:00:00 2001 From: Mathias Wulfman Date: Thu, 27 Jun 2024 14:16:53 +0200 Subject: [PATCH 1/6] :hammer: fix unflatten when processing Ordered dict --- python/jiminy_py/src/jiminy_py/tree.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/jiminy_py/src/jiminy_py/tree.py b/python/jiminy_py/src/jiminy_py/tree.py index 9b33940e8d..6d0892808a 100644 --- a/python/jiminy_py/src/jiminy_py/tree.py +++ b/python/jiminy_py/src/jiminy_py/tree.py @@ -19,6 +19,7 @@ from functools import lru_cache from itertools import chain, starmap from collections.abc import (Mapping, ValuesView, Sequence, Set) +from collections import OrderedDict from typing import ( Any, Union, Mapping as MappingT, Iterable, Iterator as Iterator, Tuple, TypeVar, Callable, Type) @@ -173,10 +174,10 @@ def _unflatten_as(data: StructNested[Any], """ data_type = type(data) if issubclass_mapping(data_type): # type: ignore[arg-type] - return data_type({ # type: ignore[call-arg] + return data_type(OrderedDict({ # type: ignore[call-arg] key: _unflatten_as(value, data_leaf_it) for key, value in data.items() # type: ignore[union-attr] - }) + })) if issubclass_sequence(data_type): # type: ignore[arg-type] return data_type(tuple( # type: ignore[call-arg] _unflatten_as(value, data_leaf_it) for value in data From 0d27efd25d9083e41a99d40f065293aae51c21a6 Mon Sep 17 00:00:00 2001 From: Mathias Wulfman Date: Thu, 27 Jun 2024 16:01:42 +0200 Subject: [PATCH 2/6] apply suggestion --- python/jiminy_py/src/jiminy_py/tree.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/python/jiminy_py/src/jiminy_py/tree.py b/python/jiminy_py/src/jiminy_py/tree.py index 6d0892808a..41c812addc 100644 --- a/python/jiminy_py/src/jiminy_py/tree.py +++ b/python/jiminy_py/src/jiminy_py/tree.py @@ -19,7 +19,6 @@ from functools import lru_cache from itertools import chain, starmap from collections.abc import (Mapping, ValuesView, Sequence, Set) -from collections import OrderedDict from typing import ( Any, Union, Mapping as MappingT, Iterable, Iterator as Iterator, Tuple, TypeVar, Callable, Type) @@ -174,10 +173,19 @@ def _unflatten_as(data: StructNested[Any], """ data_type = type(data) if issubclass_mapping(data_type): # type: ignore[arg-type] - return data_type(OrderedDict({ # type: ignore[call-arg] - key: _unflatten_as(value, data_leaf_it) - for key, value in data.items() # type: ignore[union-attr] - })) + try: + return data_type([ # type: ignore[call-arg] + (key, _unflatten_as(value, data_leaf_it)) + for key, value in data.items() # type: ignore[union-attr] + ]) + except (ValueError, RuntimeError): + # Fallback to initialisation from dict in the rare event of + # a container type not supporting initialisation from a + # sequence of key-value pairs. + return data_type({ # type: ignore[call-arg] + key: _unflatten_as(value, data_leaf_it) + for key, value in data.items() # type: ignore[union-attr] + }) if issubclass_sequence(data_type): # type: ignore[arg-type] return data_type(tuple( # type: ignore[call-arg] _unflatten_as(value, data_leaf_it) for value in data From ee3ebec62ff9dedd0352c6c02def23f9133e48cb Mon Sep 17 00:00:00 2001 From: Mathias Wulfman Date: Thu, 27 Jun 2024 16:14:06 +0200 Subject: [PATCH 3/6] :hammer: avoid redundant computation in fallback --- python/jiminy_py/src/jiminy_py/tree.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/python/jiminy_py/src/jiminy_py/tree.py b/python/jiminy_py/src/jiminy_py/tree.py index 41c812addc..4dd521bfdf 100644 --- a/python/jiminy_py/src/jiminy_py/tree.py +++ b/python/jiminy_py/src/jiminy_py/tree.py @@ -173,19 +173,15 @@ def _unflatten_as(data: StructNested[Any], """ data_type = type(data) if issubclass_mapping(data_type): # type: ignore[arg-type] + flat_items = [(key, _unflatten_as(value, data_leaf_it)) + for key, value in data.items()] # type: ignore[union-attr] try: - return data_type([ # type: ignore[call-arg] - (key, _unflatten_as(value, data_leaf_it)) - for key, value in data.items() # type: ignore[union-attr] - ]) + return data_type(flat_items) # type: ignore[call-arg] except (ValueError, RuntimeError): # Fallback to initialisation from dict in the rare event of # a container type not supporting initialisation from a # sequence of key-value pairs. - return data_type({ # type: ignore[call-arg] - key: _unflatten_as(value, data_leaf_it) - for key, value in data.items() # type: ignore[union-attr] - }) + return data_type(dict(flat_items)) # type: ignore[call-arg] if issubclass_sequence(data_type): # type: ignore[arg-type] return data_type(tuple( # type: ignore[call-arg] _unflatten_as(value, data_leaf_it) for value in data From e16355adc5746c6b0154b553265e2278c96ea237 Mon Sep 17 00:00:00 2001 From: Mathias Wulfman Date: Thu, 27 Jun 2024 16:15:17 +0200 Subject: [PATCH 4/6] :memo: add comments --- python/jiminy_py/src/jiminy_py/tree.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/jiminy_py/src/jiminy_py/tree.py b/python/jiminy_py/src/jiminy_py/tree.py index 4dd521bfdf..1fec5e8def 100644 --- a/python/jiminy_py/src/jiminy_py/tree.py +++ b/python/jiminy_py/src/jiminy_py/tree.py @@ -176,6 +176,8 @@ def _unflatten_as(data: StructNested[Any], flat_items = [(key, _unflatten_as(value, data_leaf_it)) for key, value in data.items()] # type: ignore[union-attr] try: + # Initialisation from dict cannot be the default path as `gym.spaces.Dict` + # would sort keys in this specific scenario, which must be avoided. return data_type(flat_items) # type: ignore[call-arg] except (ValueError, RuntimeError): # Fallback to initialisation from dict in the rare event of From 92fd47726a952c01b862b8e225bfb98f1a6840cd Mon Sep 17 00:00:00 2001 From: Mathias Wulfman Date: Thu, 27 Jun 2024 17:41:40 +0200 Subject: [PATCH 5/6] :rotating_light: fix linter --- python/jiminy_py/src/jiminy_py/tree.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/jiminy_py/src/jiminy_py/tree.py b/python/jiminy_py/src/jiminy_py/tree.py index 1fec5e8def..e8de3c9af5 100644 --- a/python/jiminy_py/src/jiminy_py/tree.py +++ b/python/jiminy_py/src/jiminy_py/tree.py @@ -176,8 +176,9 @@ def _unflatten_as(data: StructNested[Any], flat_items = [(key, _unflatten_as(value, data_leaf_it)) for key, value in data.items()] # type: ignore[union-attr] try: - # Initialisation from dict cannot be the default path as `gym.spaces.Dict` - # would sort keys in this specific scenario, which must be avoided. + # Initialisation from dict cannot be the default path as + # `gym.spaces.Dict` would sort keys in this specific scenario, + # which must be avoided. return data_type(flat_items) # type: ignore[call-arg] except (ValueError, RuntimeError): # Fallback to initialisation from dict in the rare event of From 8bf3465b37839417f46ddee0852362464063e398 Mon Sep 17 00:00:00 2001 From: Mathias Wulfman Date: Fri, 28 Jun 2024 15:40:13 +0200 Subject: [PATCH 6/6] :rotating_light: fix flake8 error --- python/jiminy_py/src/jiminy_py/tree.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/python/jiminy_py/src/jiminy_py/tree.py b/python/jiminy_py/src/jiminy_py/tree.py index e8de3c9af5..219029120d 100644 --- a/python/jiminy_py/src/jiminy_py/tree.py +++ b/python/jiminy_py/src/jiminy_py/tree.py @@ -173,8 +173,10 @@ def _unflatten_as(data: StructNested[Any], """ data_type = type(data) if issubclass_mapping(data_type): # type: ignore[arg-type] - flat_items = [(key, _unflatten_as(value, data_leaf_it)) - for key, value in data.items()] # type: ignore[union-attr] + flat_items = [ + (key, _unflatten_as(value, data_leaf_it)) + for key, value in data.items() # type: ignore[union-attr] + ] try: # Initialisation from dict cannot be the default path as # `gym.spaces.Dict` would sort keys in this specific scenario,