Convert node to list so that they are processed in order. #507
Conversation
| for node_name, prefix in prefixes.items(): | ||
| nodes[node_name] = nodes[node_name].with_launch_prefix(prefix) | ||
| return attr.evolve(self, nodes=frozenset(nodes.values())) | ||
| return attr.evolve(self, nodes=list(nodes.values())) |
There was a problem hiding this comment.
prefer (immutable) tuple over list
| m = m.format(node.full_name) | ||
| raise FailedToParseLaunchFile(m) | ||
| nodes = self.nodes | frozenset({node}) | ||
| nodes = self.nodes | list(node) |
There was a problem hiding this comment.
I haven't looked at the mypy output yet, but I suspect that this is the line that's causing it to fail. The set union operator doesn't make sense for a list here. This line can just be replaced by:
nodes = self.nodes + [node]There was a problem hiding this comment.
I completely misread what this line was doing. Changed it to extend the tuple
| nodes.add(node) | ||
| return attr.evolve(self, nodes=frozenset(nodes)) | ||
| nodes.append(node) | ||
| return attr.evolve(self, nodes=nodes) |
There was a problem hiding this comment.
beware: nodes is a list here; we probably want to convert it to a tuple before passing it to evolve (otherwise we'll get a crash on line 140)
| @attr.s(frozen=True, slots=True) | ||
| class LaunchConfig: | ||
| nodes: Sequence[NodeConfig] = attr.ib(default=()) | ||
| nodes: Tuple[NodeConfig, ...] = attr.ib(default=()) |
There was a problem hiding this comment.
why this change? (tuples are sequences)
There was a problem hiding this comment.
If I make it a Sequence, mypy doesn't like the '+' operation.
src/roswire/common/launch/config/launch.py:140: error: Unsupported left operand type for + ("Sequence[NodeConfig]")
Is there a way around this?
Nodes in a LaunchConfig were sets, which breaks the ordering required for starting nodelets after nodelet managers.