Type checking with mypy

[haudren-woven]

Bug report

I am sorry to open an issue in such short order, but as part of my work at Woven Planet, I'm trying to use the ROS2 launch system. Unfortunately, I have found that it type information is currently unusable from an external package, thus I'd like to improve it.

Required Info:

• Operating System:
  • Ubuntu 18.04
• Installation type:
  • Source
• Version or commit hash:
  • 6783ca8
• DDS implementation:
  • CycloneDDS
• Client library (if applicable):
  • N/A

Steps to reproduce issue

1. Create a snippet with this content:

import launch

context = launch.LaunchContext()
reveal_type(context)

2. Install mypy
3. Run the following:

MYPYPATH=$PATH_TO_WS/install/launch/lib/python3.8/site-packages/ mypy -m launch

(mypy does not use PYTHONPATH by default)

Expected behavior

test.py:4: note: Revealed type is 'launch.launch_context.LaunchContext'

Actual behavior

test.py:1: error: Cannot find implementation or library stub for module named 'launch'
test.py:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
test.py:4: note: Revealed type is 'Any'

Feature request

Feature description

I'd like to make the type checking in launch (and potentially other ROS2 core libraries) a little bit stricter. This would allow to enforce a quality standard, and root out bugs at the base. It would also make the code easier to use from the outside, since type checking would prevent some common bugs in launch files, such as using a substitution where a condition is expected. Since type-checking is strictly opt-in, there would be no impact on the broader community.

Implementation considerations

• I have already implemented adding the py.typed file to mark the launch package as typed to the broader system, and can submit a PR promptly
• However, there are many type-checking issues in the current codebase, including bugs. For example, mypy reported that this function does not have a return statement, contradicting its signature. Similarly, this statement has invalid type syntax. Probably someone meant Tuple? In total mypy reports 224 errors in 47 files.
• Thus, I think we should also add a CI test to check the type information using mypy. This is I think very straightforward using ament_mypy in the same way that you are currently using ament_pep257 and others.

I can take care of the above points, but I'd like to make sure that the maintainers agree with this approach before making a PR full of tiny changes.

Thank you very much!

[ros-discourse]

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/python-type-checking-in-ros2/28507/1

[haudren-woven]

@adityapande-1995 @wjwwood : Any opinions on this? Would you be opposed to some PRs that would improve the situation?

[clalancette]

In short, I think we would welcome additions to the typing (there are already some open PRs for this).

I'm also interested in adding ament_mypy by default, but before we do that I'd want to get a good idea on how much extra time it adds to CI. Our CI already takes many hours, so I don't want to increase that too much (especially on Windows).

[haudren-woven]

I can add that :+1: In my personal testing, running mypy itself only takes a few seconds.

Found 221 errors in 47 files (checked 121 source files)
mypy --follow-imports=silent src/launch/launch  1.07s user 0.07s system 98% cpu 1.163 total