From f2c4c22ef58cc5ff2ffb3db86b35c896593ace93 Mon Sep 17 00:00:00 2001 From: Nic Ma Date: Fri, 26 Aug 2022 12:55:53 +0800 Subject: [PATCH 1/4] [DLMED] add support to set nested keys Signed-off-by: Nic Ma --- monai/bundle/config_parser.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/monai/bundle/config_parser.py b/monai/bundle/config_parser.py index aa577c6b75..4fe469711e 100644 --- a/monai/bundle/config_parser.py +++ b/monai/bundle/config_parser.py @@ -136,7 +136,7 @@ def __getitem__(self, id: Union[str, int]): config = config[indexing] return config - def __setitem__(self, id: Union[str, int], config: Any): + def __setitem__(self, id: Union[str, int], config: Any, recursive: bool = True): """ Set config by ``id``. Note that this method should be used before ``parse()`` or ``get_parsed_content()`` to ensure the updates are included in the parsed content. @@ -154,9 +154,25 @@ def __setitem__(self, id: Union[str, int], config: Any): self.ref_resolver.reset() return keys = str(id).split(ID_SEP_KEY) - # get the last parent level config item and replace it - last_id = ID_SEP_KEY.join(keys[:-1]) - conf_ = self[last_id] + conf_ = self.get() + if recursive: + if conf_ is None: + conf_ = [] if keys[0] == "0" else {} + for i, k in enumerate(keys[:-1]): + if isinstance(conf_, dict): + if k not in conf_: + conf_[k] = [] if keys[i + 1] == "0" else {} + if isinstance(conf_, list): + if int(k) == len(conf_): + conf_[int(k)] = [] if keys[i + 1] == "0" else {} + elif int(k) > len(conf_): + raise ValueError("can only set new item with index = the max index + 1.") + conf_ = conf_[k if isinstance(conf_, dict) else int(k)] + else: + # get the last parent level config item and replace it + last_id = ID_SEP_KEY.join(keys[:-1]) + conf_ = self[last_id] + indexing = keys[-1] if isinstance(conf_, dict) else int(keys[-1]) conf_[indexing] = config self.ref_resolver.reset() From 7c37d01d82c996245a0c571524c885eedf3eb0cc Mon Sep 17 00:00:00 2001 From: Nic Ma Date: Fri, 26 Aug 2022 13:19:51 +0800 Subject: [PATCH 2/4] [DLMED] add tests Signed-off-by: Nic Ma --- monai/bundle/config_parser.py | 2 +- tests/test_config_parser.py | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/monai/bundle/config_parser.py b/monai/bundle/config_parser.py index 4fe469711e..b2a3c3d0e4 100644 --- a/monai/bundle/config_parser.py +++ b/monai/bundle/config_parser.py @@ -164,7 +164,7 @@ def __setitem__(self, id: Union[str, int], config: Any, recursive: bool = True): conf_[k] = [] if keys[i + 1] == "0" else {} if isinstance(conf_, list): if int(k) == len(conf_): - conf_[int(k)] = [] if keys[i + 1] == "0" else {} + conf_.append([] if keys[i + 1] == "0" else {}) elif int(k) > len(conf_): raise ValueError("can only set new item with index = the max index + 1.") conf_ = conf_[k if isinstance(conf_, dict) else int(k)] diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py index 3fa581b6b0..21e356c077 100644 --- a/tests/test_config_parser.py +++ b/tests/test_config_parser.py @@ -123,9 +123,18 @@ def test_parse(self, config, expected_ids, output_types): self.assertEqual(trans, parser.get_parsed_content(id="transform#transforms#0")) self.assertEqual(trans, parser.get_parsed_content(id="transform#transforms#0", lazy=True)) self.assertNotEqual(trans, parser.get_parsed_content(id="transform#transforms#0", lazy=False)) - # test nested id + # test new nested id + parser["transform#transforms#1#keys"] = "fake_key1" + parser["transform#other_transforms#0#keys"] = "fake_key2" + self.assertEqual(parser.get(id="transform#transforms#1#keys"), "fake_key1") + self.assertEqual(parser.get(id="transform#other_transforms#0#keys"), "fake_key2") + # remove temp fake data + parser["transform#transforms"].pop(-1) + parser["transform"].pop("other_transforms") + # test update nested id parser["transform#transforms#0#keys"] = "label2" self.assertEqual(parser.get_parsed_content(id="transform#transforms#0").keys[0], "label2") + for id, cls in zip(expected_ids, output_types): self.assertTrue(isinstance(parser.get_parsed_content(id), cls)) # test root content From 16c45373c7100932c81ff647fef490dabc8290b4 Mon Sep 17 00:00:00 2001 From: Nic Ma Date: Fri, 26 Aug 2022 17:43:30 +0800 Subject: [PATCH 3/4] [DLMED] update accoridng to comments Signed-off-by: Nic Ma --- monai/bundle/config_parser.py | 44 +++++++++++++++++++---------------- tests/test_config_parser.py | 6 ++--- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/monai/bundle/config_parser.py b/monai/bundle/config_parser.py index b2a3c3d0e4..da5a7e5895 100644 --- a/monai/bundle/config_parser.py +++ b/monai/bundle/config_parser.py @@ -136,7 +136,7 @@ def __getitem__(self, id: Union[str, int]): config = config[indexing] return config - def __setitem__(self, id: Union[str, int], config: Any, recursive: bool = True): + def __setitem__(self, id: Union[str, int], config: Any): """ Set config by ``id``. Note that this method should be used before ``parse()`` or ``get_parsed_content()`` to ensure the updates are included in the parsed content. @@ -154,24 +154,9 @@ def __setitem__(self, id: Union[str, int], config: Any, recursive: bool = True): self.ref_resolver.reset() return keys = str(id).split(ID_SEP_KEY) - conf_ = self.get() - if recursive: - if conf_ is None: - conf_ = [] if keys[0] == "0" else {} - for i, k in enumerate(keys[:-1]): - if isinstance(conf_, dict): - if k not in conf_: - conf_[k] = [] if keys[i + 1] == "0" else {} - if isinstance(conf_, list): - if int(k) == len(conf_): - conf_.append([] if keys[i + 1] == "0" else {}) - elif int(k) > len(conf_): - raise ValueError("can only set new item with index = the max index + 1.") - conf_ = conf_[k if isinstance(conf_, dict) else int(k)] - else: - # get the last parent level config item and replace it - last_id = ID_SEP_KEY.join(keys[:-1]) - conf_ = self[last_id] + # get the last parent level config item and replace it + last_id = ID_SEP_KEY.join(keys[:-1]) + conf_ = self[last_id] indexing = keys[-1] if isinstance(conf_, dict) else int(keys[-1]) conf_[indexing] = config @@ -192,15 +177,34 @@ def get(self, id: str = "", default: Optional[Any] = None): except (KeyError, IndexError): # Index error for integer indexing return default - def set(self, config: Any, id: str = ""): + def set(self, config: Any, id: str = "", recursive: bool = True): """ Set config by ``id``. Args: config: config to set at location ``id``. id: id to specify the expected position. See also :py:meth:`__setitem__`. + recursive: if the nested id doesn't exist, whether to recursively create the nested items in the config. + default to `True`. if the id is int number, will treat it as index of list: + 1. if no list exists, only allow `index = 0` to create a list with one item, like: `id="key1#0#key2"`. + 2. if the list exists, allow `index <= length` to update existing item or append a new item to the end. """ + keys = str(id).split(ID_SEP_KEY) + conf_ = self.get() + if recursive: + if conf_ is None: + self.config = conf_ = [] if keys[0] == "0" else {} # type: ignore + for i, k in enumerate(keys[:-1]): + if isinstance(conf_, dict): + if k not in conf_: + conf_[k] = [] if keys[i + 1] == "0" else {} + if isinstance(conf_, list): + if int(k) == len(conf_): + conf_.append([] if keys[i + 1] == "0" else {}) + elif int(k) > len(conf_): + raise ValueError("can only set new item with index = the max index + 1.") + conf_ = conf_[k if isinstance(conf_, dict) else int(k)] self[id] = config def update(self, pairs: Dict[str, Any]): diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py index 21e356c077..61992a2df4 100644 --- a/tests/test_config_parser.py +++ b/tests/test_config_parser.py @@ -124,9 +124,9 @@ def test_parse(self, config, expected_ids, output_types): self.assertEqual(trans, parser.get_parsed_content(id="transform#transforms#0", lazy=True)) self.assertNotEqual(trans, parser.get_parsed_content(id="transform#transforms#0", lazy=False)) # test new nested id - parser["transform#transforms#1#keys"] = "fake_key1" - parser["transform#other_transforms#0#keys"] = "fake_key2" - self.assertEqual(parser.get(id="transform#transforms#1#keys"), "fake_key1") + parser.set("fake_key1", "transform#transforms#2#keys", True) + parser.set("fake_key2", "transform#other_transforms#0#keys", True) + self.assertEqual(parser.get(id="transform#transforms#2#keys"), "fake_key1") self.assertEqual(parser.get(id="transform#other_transforms#0#keys"), "fake_key2") # remove temp fake data parser["transform#transforms"].pop(-1) From 10f7356faad548bda093c72c32b95f2978f6f904 Mon Sep 17 00:00:00 2001 From: Nic Ma Date: Fri, 26 Aug 2022 23:20:02 +0800 Subject: [PATCH 4/4] [DLMED] simplify according to comments Signed-off-by: Nic Ma --- monai/bundle/config_parser.py | 18 +++++------------- tests/test_config_parser.py | 7 ++----- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/monai/bundle/config_parser.py b/monai/bundle/config_parser.py index da5a7e5895..e15c451b3b 100644 --- a/monai/bundle/config_parser.py +++ b/monai/bundle/config_parser.py @@ -185,25 +185,17 @@ def set(self, config: Any, id: str = "", recursive: bool = True): config: config to set at location ``id``. id: id to specify the expected position. See also :py:meth:`__setitem__`. recursive: if the nested id doesn't exist, whether to recursively create the nested items in the config. - default to `True`. if the id is int number, will treat it as index of list: - 1. if no list exists, only allow `index = 0` to create a list with one item, like: `id="key1#0#key2"`. - 2. if the list exists, allow `index <= length` to update existing item or append a new item to the end. + default to `True`. for the nested id, only support `dict` for the missing section. """ keys = str(id).split(ID_SEP_KEY) conf_ = self.get() if recursive: if conf_ is None: - self.config = conf_ = [] if keys[0] == "0" else {} # type: ignore - for i, k in enumerate(keys[:-1]): - if isinstance(conf_, dict): - if k not in conf_: - conf_[k] = [] if keys[i + 1] == "0" else {} - if isinstance(conf_, list): - if int(k) == len(conf_): - conf_.append([] if keys[i + 1] == "0" else {}) - elif int(k) > len(conf_): - raise ValueError("can only set new item with index = the max index + 1.") + self.config = conf_ = {} # type: ignore + for k in keys[:-1]: + if isinstance(conf_, dict) and k not in conf_: + conf_[k] = {} conf_ = conf_[k if isinstance(conf_, dict) else int(k)] self[id] = config diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py index 61992a2df4..1636f9818a 100644 --- a/tests/test_config_parser.py +++ b/tests/test_config_parser.py @@ -124,12 +124,9 @@ def test_parse(self, config, expected_ids, output_types): self.assertEqual(trans, parser.get_parsed_content(id="transform#transforms#0", lazy=True)) self.assertNotEqual(trans, parser.get_parsed_content(id="transform#transforms#0", lazy=False)) # test new nested id - parser.set("fake_key1", "transform#transforms#2#keys", True) - parser.set("fake_key2", "transform#other_transforms#0#keys", True) - self.assertEqual(parser.get(id="transform#transforms#2#keys"), "fake_key1") - self.assertEqual(parser.get(id="transform#other_transforms#0#keys"), "fake_key2") + parser.set("fake_key", "transform#other_transforms#keys", True) + self.assertEqual(parser.get(id="transform#other_transforms#keys"), "fake_key") # remove temp fake data - parser["transform#transforms"].pop(-1) parser["transform"].pop("other_transforms") # test update nested id parser["transform#transforms#0#keys"] = "label2"