Add normalize_remap_rule and types #173
Conversation
Adds remap rule types to and utility functions to type check and normalize remap rules. Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
sloretz
added
enhancement
|
|
||
| SomeRemapRule = Tuple[SomeSubstitutionsType, SomeSubstitutionsType] | ||
| SomeRemapRules = Iterable[SomeRemapRule] | ||
| RemapRule = Tuple[Tuple[Substitution, ...], Tuple[Substitution, ...]] |
There was a problem hiding this comment.
This is Tuple[Tuple[ instead of Tuple[List[ to make a normalized remap rule immutable. (Edit: well, the Substition instances in the Tuple aren't immutable, so I guess it's just less mutable)
There isn't anything called description yet, so currently I'm thinking a launch_ros.description.ComposableNode instance is an immutable description of some end state. Unlike an action instance which is only performed once, a description could be given to multiple actions. Defining the type as Tuple is a way to prevent remap rules on it from being modified.
There was a problem hiding this comment.
Re: Tuple vs List, immutable is fine by me.
Re: launch_ros.description.*, is there a description base class for these? If not, I had imagined they would not be in a subpackage together. If so, then I think a module called launch_ros.description with a subpackage called launch_ros.descriptions.* would be most appropriate.
Otherwise, I think launch_ros.ComposableNode is just fine.
There was a problem hiding this comment.
Re: launch_ros.description.*, is there a description base class for these? If not, I had imagined they would not be in a subpackage together.
Roger, launch_ros.ComposableNode it is
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
|
Added tests in 1e0616c CI (testing
|
I want to reuse the types and type checking on arguments to
NodeinComposableNode. This PR does that for theremappingsargument. I'd like to do the same thing forparameters, but thought I would split it up to get feedback sooner.