Discussion on future improvements

[AlexAntn]

With the first draft of the modifiers library being almost merged, we are getting to a point where we can look into what to change/add to the library. For this, it is important to know what we actually need, or would be useful to have.

This issue will work as a collection/discussion of improvement ideas to consider from now on.

[GrmanRodriguez]

One possible improvement is on how we write modifications.

As @AlexAntn knows the first approach was adding a new parameter for every possible change:

modifier.modify(density_multiplier=1,length_multiplier=1)

However as soon as we began adding more features the call to the modify function became bigger and bigger.

For this reason we decided to use a dict and input everything there. The solution was a bit rough, using an array as the key to specify whether the change is absolute or scaled:

modification = {
    "density":[1,true],
    "length":[1,false]
}
modifier.modify(modification)

While the call to the modify function is now shorter, we now have to write the dict, and know the specific syntax that the modify function requires which is clunky. To this end, @CarlottaSartore added the enum-like Modification class:

modifications= {}
modifications[utils.geometry.Modification.DIMENSION]=[0.2,utils.geometry.Modification.ABSOLUTE]

While this helps with not needing to memorize, I still feel that this way of writing the modifications is not super clear and not very pythonic.

My proposal is to make the Modification class a full class, with constructors and methods, something like this:

modification = Modification()
modification.add_density(1,a_new_enum.ABSOLUTE)
modification.add_length(1,a_new_enum.RELATIVE)
modifier.modify(modification)

We could also add different constructors based on dict, .ini files, or anything we wish:

modification_dict = {
    "density":[1,true],
    "length":[1,false]
}
modification_1 = Modification.from_dict(modification_dict)
ini_filepath='/path/to/ini/file'
modification_2 = Modification.from_file(ini_filepath)

We get the best of both worlds: syntax that is concise but also legible and does not require knowing it by memory.

[AlexAntn]

One other improvement to consider (suggested by @CarlottaSartore ) is to automatically calculate the offsets between links/joints. This would calculate the offsets based on the parent URDF, with the possibility of specifying the offsets "by hand" instead.

[AlexAntn]

I think this issue is no longer useful since we are having meetings to discuss the next steps and opening the issues as we discuss them. The modifications discussed here have been implemented, so closing.