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

Feature Request: Return Dictionary in Python #155

Open
peterdavidfagan opened this issue Oct 27, 2023 · 6 comments
Open

Feature Request: Return Dictionary in Python #155

peterdavidfagan opened this issue Oct 27, 2023 · 6 comments

Comments

@peterdavidfagan
Copy link

peterdavidfagan commented Oct 27, 2023

Thank you for creating such a useful library.

A feature request I had was to incorporate the returning of Python dictionaries. The motivation for this is to enable iteration through parameters in Python without having to explicitly using full parameter names to access values.

I currently have a yaml file containing multiple sensor configs, rather than having to individually create a subscriber for each I want to have a generic register_sensors method that iterates through sensors and their configuration parameters in order to create subscriptions. To accomplish this I would like to have my parameters in a dictionary where nested parameter structure is respected.

self.param_listener = params.ParamListener(self)
self.param_dict = self.param_listener.get_params().to_dict()

Maybe this functionality already exists? I couldn't find it on an initial pass through the library.

@pac48
Copy link
Collaborator

pac48 commented Oct 31, 2023

@peterdavidfagan Thanks for submitting the issue. There are two ways to go about adding this functionality. 1) modify the code generation logic and Python template or 2) try to make use of Python's flexibility to construct the dictionary dynamically. I would prefer option two if possible, that a method can simply be added to the Jinja template. The following implementation should work.

    def to_dict(self):
        out = {}

        def to_dict_internal(instance, base_name):
            if isinstance(instance, (str, numbers.Number, list)):
                out.update({base_name: instance})
            else:
                data = [attr for attr in dir(instance) if not callable(getattr(instance, attr)) and not attr.startswith("__") and attr is not "stamp_"]
                for attr in data:
                    to_dict_internal(getattr(instance, attr), ("" if base_name == "" else base_name + ".") + attr)

        to_dict_internal(self, "")

        return out

Unfortunately, I am not aware of a simpler way to achieve this. Plus, it is unclear if there is a missing edge case in filtering the output of the dir method. If you or anyone has a suggestion, I'd be interested to get feedback.

@peterdavidfagan
Copy link
Author

peterdavidfagan commented Nov 1, 2023

Thanks @pac48, I agree with your suggestion (2), this makes sense and thanks for sharing your code for this. I had started a similar method locally as a util, I'll look to combine this with your code and open a pull request with this method if this works for you?

I'll look to add the method to the ParamListener.

@pac48
Copy link
Collaborator

pac48 commented Nov 1, 2023

@peterdavidfagan Awesome, that sounds like a good plan. I think it might be better to add the method to the Params (below this line ) class instead. That way, the syntax you proposed will work.

param_dict = param_listener.get_params().to_dict()

@peterdavidfagan
Copy link
Author

That way, the syntax you proposed will work.

Makes sense, I'll hopefully get to this item before the end of this week.

@pac48
Copy link
Collaborator

pac48 commented Dec 17, 2023

@peterdavidfagan I just wanted to ping you about this request. If you were able to make the change feel free to open a PR.

@peterdavidfagan
Copy link
Author

peterdavidfagan commented Dec 17, 2023

Hey @pac48,

I haven't opened pr yet as I did not finish these changes, but I have been meaning to follow up on this. Currently we are using the policy deployment code in our lab but have been reading yaml files directly via Python. At the moment, I am focused on completing a simulation env for a benchmark, in the new year I have a deadline for policy deployment baselines at which point I plan to update the moveit python library policy deployment code + tutorials. I will look to complete these changes when I am pushing this code.

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

No branches or pull requests

2 participants