ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
126 stars 144 forks source link

Remove the import of Literal from entity.py. #694

Closed clalancette closed 1 year ago

clalancette commented 1 year ago

Literal is not supported in Python 3.6, which is what is in RHEL-8. Just remove this bit for now, since I don't actually believe it is needed anyway.

This particular issue was introduced in #679 . FYI @haudren-woven @methylDragon

clalancette commented 1 year ago

CI:

clalancette commented 1 year ago

The unstable Windows build has been fixed elsewhere (https://github.com/ros2/rclcpp/pull/2117). The unstable RHEL tests are known issues (but likely to go away when we switch to RHEL-9). So I'm going to go ahead and merge this in.

haudren-woven commented 1 year ago

So in the context of launch, this ended up being fine but I needed the cast. The problem with removing literal and associated overloads is that you’re relaxing the type contract: when optional is False, this function has to return the type. In a few places of the code this shows up, but nothing that a couple of is not None checks can’t solve.

Let’s just hope we can stop supporting Python 3.6 soonish…