Skip to content
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.

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

5804 Simplify the configparser usage to get parsed attributes easily #5813

Conversation

Shadow-Devil
Copy link
Contributor

@Shadow-Devil Shadow-Devil commented Jan 5, 2023

part of #5804.

Description

This adds a simple attribute access for ConfigParser so

from monai.bundle import ConfigParser
parser = ConfigParser({"a":"b"})
a = parser.get_parsed_content("a")

can be rewritten to

from monai.bundle import ConfigParser
parser = ConfigParser({"a":"b"})
a = parser.a

This only works for variables/methods that are not included in ConfigParser so e.g. parser.config would still get the whole config.
This PR only supports shallow attribute accesses, since the returned value will be a dict where you need to use ["key"] to get the content.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • New tests added to cover the changes.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.

monai/bundle/config_parser.py Outdated Show resolved Hide resolved
…ibute (not nested).

Signed-off-by: Felix Schnabel <f.schnabel@tum.de>
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

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

Thanks, it looks good to me.

@Shadow-Devil
Copy link
Contributor Author

Shadow-Devil commented Jan 6, 2023

One Idea was to change the get_config of ConfigItem like this:

class ConfigItem:
...
    def get_config(self) -> Any:
        """
        Get the config content of current config item.
        If the content is a dict it will wrap it with `DictWrapper` to allow attribute like access.

        """
        
        return self.config if not isinstance(self.config, dict) else DictWrapper(self.config)

class DictWrapper(UserDict):
    """
    Wrapper class to support attribute-style access to the dict items.
    """

    def __getattr__(self, key: str) -> Any:
        if key in self.data:
            item = self.data[key]
            return item if not isinstance(item, dict) else DictWrapper(item)
        raise AttributeError(f"'{self.__class__.__name__}' object has no attribute '{key}'")

This implementation wraps each dict into this DictWrapper which will in itself wrap each dict again into a DictWrapper.

@Shadow-Devil
Copy link
Contributor Author

The problem with this implementation is, that nested content is not parsed/instantiated...

@wyli
Copy link
Contributor

wyli commented Jan 6, 2023

I'll merge this PR for now... for the nested cases, perhaps we'll introduce a new class for ConfigParser.config? do you plan to continue working on this @Shadow-Devil? otherwise @Nic-Ma and I can take over and enhance. we are fine with either way.

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli enabled auto-merge (squash) January 6, 2023 13:03
@Shadow-Devil
Copy link
Contributor Author

Thanks @wyli, I think it would be better if you continue since I'm not sure how this new class should look like and there are some design decisions to make, which should be in line with the rest of the project. If you have a general plan on how this should look like, I'm happy to implement it :)

@Nic-Ma Nic-Ma disabled auto-merge January 6, 2023 13:30
@wyli
Copy link
Contributor

wyli commented Jan 6, 2023

/build

Shadow-Devil and others added 3 commits January 6, 2023 15:06
Signed-off-by: Felix Schnabel <f.schnabel@tum.de>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli
Copy link
Contributor

wyli commented Jan 6, 2023

/build

@wyli wyli enabled auto-merge (squash) January 6, 2023 14:12
@Nic-Ma Nic-Ma disabled auto-merge January 6, 2023 14:53
@wyli wyli enabled auto-merge (squash) January 6, 2023 14:59
@wyli wyli merged commit 9efd54d into Project-MONAI:dev Jan 6, 2023
@Shadow-Devil Shadow-Devil deleted the feature_config_parser_access_via_attribute branch January 23, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants