generate_parameter_library icon indicating copy to clipboard operation
generate_parameter_library copied to clipboard

Feature Request: Return Dictionary in Python

Open peterdavidfagan opened this issue 2 years ago • 6 comments
trafficstars

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.

peterdavidfagan avatar Oct 27 '23 08:10 peterdavidfagan

@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.

pac48 avatar Oct 31 '23 23:10 pac48

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.

peterdavidfagan avatar Nov 01 '23 07:11 peterdavidfagan

@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()

pac48 avatar Nov 01 '23 16:11 pac48

That way, the syntax you proposed will work.

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

peterdavidfagan avatar Nov 02 '23 17:11 peterdavidfagan

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

pac48 avatar Dec 17 '23 00:12 pac48

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.

peterdavidfagan avatar Dec 17 '23 07:12 peterdavidfagan