Improve type annotations for ament_mypy #5575

leander-dsouza · 2025-10-02T12:34:17Z

Basic Info

Info	Please fill out this column
Ticket(s) this relates	#5565
Primary OS tested on	Ubuntu
Robotic platform tested on	N/A
Does this PR contain AI-generated software?	No
Was this PR description generated by AI software?	No

Description of contribution in a few bullet points

Fix incorrect methods and static definitions.
Drop transform3d and graphviz from mypy ignored packages from warnings generated in the logs.

Description of documentation updates required from your changes

N/A

Description of how this change was tested

Performed linting validation using pre-commit run --all

Future work that may be required in bullet points

I do not think we should remove the --strict flag or the ament_mypy linting yet. As it catches genuine wrong static definitions that were inserted earlier.

For Maintainers:

Check that any new parameters added are updated in docs.nav2.org
Check that any significant change is added to the migration guide
Check that any new features OR changes to existing behaviors are reflected in the tuning guide
Check that any new functions have Doxygen added
Check that any new features have test coverage
Check that any new plugins is added to the plugins page
If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists
Should this be backported to current distributions? If so, tag with backport-*.

codecov · 2025-10-02T13:09:48Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

SteveMacenski · 2025-10-02T17:36:51Z

nav2_loopback_sim/nav2_loopback_sim/loopback_simulator.py

                Twist,
                'cmd_vel', self.cmdVelCallback, 10)
        else:
-            self.cmd_vel_sub = self.create_subscription(


It is because the same variable is used as a redefinition. Therefore, mypy expects a Twist definition instead of TwistStamped

$ ament_mypy --config tools/pyproject.toml nav2_loopback_sim nav2_loopback_sim/nav2_loopback_sim/loopback_simulator.py:131:17: error: Argument 1 to "create_subscription" of "Node" has incompatible type "type[TwistStamped]"; expected "type[Twist]" [arg-type]

This supports both - can we make the type a union?

I have renamed the variable as a Union - self.cmd_vel_sub: Union[Subscription[Twist], Subscription[TwistStamped]].

SteveMacenski · 2025-10-02T17:37:18Z

nav2_loopback_sim/nav2_loopback_sim/utils.py



 def transformStampedToMatrix(transform: Transform) -> np.ndarray[np.float64, np.dtype[np.float64]]:
-    translation = transform.transform.translation


I believe this is correct https://docs.ros.org/en/noetic/api/geometry_msgs/html/msg/TransformStamped.html

Yes, you are correct.
The linter was flagging it because the functional parameter was incorrectly defined as Transform instead of TransformStamped.

Rename the function then so it is accurate to type?

The function is already named as transformStampedToMatrix.
I have renamed the function parameter from transform: Transform to transform: TransformStamped.

The GitHub UI in this comment does not seem to show this change.
This change can be seen in the Files Tab section.

SteveMacenski · 2025-10-02T17:38:07Z

nav2_simple_commander/nav2_simple_commander/footprint_collision_checker.py

        y1 = 0.0

-        x0, y0 = self.worldToMapValidated(footprint.points[0].x, footprint.points[0].y)
+        footprint_points = cast(list[Point32], footprint.points)


Can we handle this better in the actual worldToMapValidated method?

I have dropped all the cast-related fixes in this PR, as this does not seem necessary.

The CI seems to now pass without any changes to the ament_mypy configuration.
There is a significant disconnect when I use the devcontainer image when running the ament_mypy linting fixes vs testing in CI using ros-tooling/action-ros-lint.

SteveMacenski · 2025-10-02T17:40:08Z

nav2_simple_commander/nav2_simple_commander/robot_navigator.py

        if not self.goal_handle or not self.goal_handle.accepted:
            self.error('Get path was rejected!')
-            self.status = GoalStatus.UNKNOWN
+            self.status = GoalStatus.STATUS_UNKNOWN


? I think it is unknown

I have compared the message definition to the following documentation page, and it suggests using STATUS_UNKNOWN.

SteveMacenski · 2025-10-02T17:40:51Z

nav2_system_tests/src/system/nav_through_poses_tester_error_msg_node.py

        ] = ActionClient(self, NavigateThroughPoses, 'navigate_through_poses')

-        self.controller_param_cli: Client[SetParameters.Request, SetParameters.Response] = \
+        self.controller_param_cli: Client[SetParameters_Request, SetParameters_Response] = \


Why change to this _ format from . format in so many places?

I have dropped this service change as well.

This is again due to the disconnect between running ament_mypy locally vs running it in CI.

SteveMacenski · 2025-10-02T17:41:53Z

Overall I'm surprised by the number of functional changes that have been made in this PR. Are these verifiable errors? Do those demos simply not work?

Signed-off-by: Leander Stephen D'Souza <[email protected]>

leander-dsouza · 2025-10-03T10:44:35Z

Overall I'm surprised by the number of functional changes that have been made in this PR. Are these verifiable errors? Do those demos simply not work?

There seems to be something weird going on with ament_mypy CI, as it momentarily failed yesterday without any changes to main, and it fixed itself hours later.

I am not sure if my environment is broken, but there is a significant mismatch in what the CI reports versus the local pre-commit testing with the ament linters.

In this PR, I have mainly focused on porting over significant inaccuracies from the errors I see in my local environment.
You can verify the mismatch by simply running pre-commit run --all in your workspace.

SteveMacenski · 2025-10-03T14:59:57Z

nav2_simple_commander/nav2_simple_commander/example_route.py

    pose = PoseStamped()
-    pose.pose.position.x = pt.x
-    pose.pose.position.y = pt.y
+    pose.pose.position.x = pt.position.x


Pose has a field position

The functional parameter was incorrectly declared and is now renamed as pt: Point.

SteveMacenski · 2025-10-03T15:00:51Z

nav2_loopback_sim/nav2_loopback_sim/utils.py



 def transformStampedToMatrix(transform: Transform) -> np.ndarray[np.float64, np.dtype[np.float64]]:
-    translation = transform.transform.translation


Rename the function then so it is accurate to type?

SteveMacenski · 2025-10-03T15:01:09Z

nav2_loopback_sim/nav2_loopback_sim/loopback_simulator.py

                Twist,
                'cmd_vel', self.cmdVelCallback, 10)
        else:
-            self.cmd_vel_sub = self.create_subscription(


This supports both - can we make the type a union?

Signed-off-by: Leander Stephen D'Souza <[email protected]>

leander-dsouza · 2025-10-06T10:31:06Z

I have added a new commit to the PR, fixing some incorrect paths from nav2_msgs.
This is motivated by some of the fixes done in #5565.

I would like to request @ajtudela for a short review in hopes that it may fix the ament_mypy CI issues in his PR.

ajtudela · 2025-10-06T11:09:49Z

I have added a new commit to the PR, fixing some incorrect paths from nav2_msgs. This is motivated by some of the fixes done in #5565.

I would like to request @ajtudela for a short review in hopes that it may fix the ament_mypy CI issues in his PR.

LGTM. I have similar changes in the Following PR.

For example, this doesnt pass the mypy pre-commit:

client: Client[GetState.Request, GetState.Response] = \
self.node.create_client(GetState, f'{node_name}/get_state')

I have to add this comment:

client: Client[GetState.Request, GetState.Response] = (  # type: ignore[name-defined]
self.node.create_client(GetState, f'{node_name}/get_state')  # type: ignore[arg-type]

But with the comments, it didn't pass the mypy in the CI.

leander-dsouza · 2025-10-06T11:39:45Z

LGTM. I have similar changes in the Following PR.

Thank you for the initial review :)

For example, this doesnt pass the mypy pre-commit:
But with the comments, it didn't pass the mypy in the CI.

There seems to be a disconnect in running ament_mypy locally, and separately in the CI, hence the CI does not need these ignored comments.

That is why the CircleCI builds show the unused-ignore error messages.

Signed-off-by: Leander Stephen D'Souza <[email protected]>

ajtudela · 2025-10-06T19:17:07Z

LGTM. I have similar changes in the Following PR.

Thank you for the initial review :)

For example, this doesnt pass the mypy pre-commit:
But with the comments, it didn't pass the mypy in the CI.

There seems to be a disconnect in running ament_mypy locally, and separately in the CI, hence the CI does not need these ignored comments.

That is why the CircleCI builds show the unused-ignore error messages.

I'm unable to pass the precommit stage if those comments aren't included.

* Enable tools directory to be mypy compliant. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Enable nav2_system_tests to be mypy compliant. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Enable nav2_docking to be mypy compliant. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Enable nav2_simple_commander to be mypy compliant. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Enable nav2_loopback_sim to be mypy compliant. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Removed unused ignores for packages for mypy compliance. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Added nav2_msgs path fixes to mypy compliance. Signed-off-by: Leander Stephen D'Souza <[email protected]> --------- Signed-off-by: Leander Stephen D'Souza <[email protected]>

leander-dsouza marked this pull request as ready for review October 2, 2025 13:10

SteveMacenski reviewed Oct 2, 2025

View reviewed changes

leander-dsouza added 3 commits October 3, 2025 09:10

Enable tools directory to be mypy compliant.

ddaf668

Signed-off-by: Leander Stephen D'Souza <[email protected]>

Enable nav2_system_tests to be mypy compliant.

6637555

Signed-off-by: Leander Stephen D'Souza <[email protected]>

Enable nav2_docking to be mypy compliant.

d0743e2

Signed-off-by: Leander Stephen D'Souza <[email protected]>

leander-dsouza force-pushed the ament-mypy-fixes branch from 700d694 to 972e26e Compare October 3, 2025 09:51

leander-dsouza marked this pull request as draft October 3, 2025 09:52

leander-dsouza changed the title ~~Fix the ament_mypy workflow~~ Improve type annotations for ament_mypy Oct 3, 2025

leander-dsouza marked this pull request as ready for review October 3, 2025 10:30

leander-dsouza requested a review from SteveMacenski October 3, 2025 10:44

SteveMacenski requested changes Oct 3, 2025

View reviewed changes

leander-dsouza added 3 commits October 6, 2025 06:38

Enable nav2_simple_commander to be mypy compliant.

72a2d87

Signed-off-by: Leander Stephen D'Souza <[email protected]>

Enable nav2_loopback_sim to be mypy compliant.

499fef6

Signed-off-by: Leander Stephen D'Souza <[email protected]>

Removed unused ignores for packages for mypy compliance.

0c53bb5

Signed-off-by: Leander Stephen D'Souza <[email protected]>

leander-dsouza force-pushed the ament-mypy-fixes branch from 45f9292 to 0c53bb5 Compare October 6, 2025 06:39

leander-dsouza requested a review from SteveMacenski October 6, 2025 07:00

leander-dsouza force-pushed the ament-mypy-fixes branch 2 times, most recently from 043f242 to b4d41fd Compare October 6, 2025 11:31

Added nav2_msgs path fixes to mypy compliance.

6ba5ab7

Signed-off-by: Leander Stephen D'Souza <[email protected]>

leander-dsouza force-pushed the ament-mypy-fixes branch from b4d41fd to 6ba5ab7 Compare October 6, 2025 12:04

SteveMacenski approved these changes Oct 6, 2025

View reviewed changes

SteveMacenski merged commit 269d480 into ros-navigation:main Oct 6, 2025
16 checks passed



		def transformStampedToMatrix(transform: Transform) -> np.ndarray[np.float64, np.dtype[np.float64]]:
		translation = transform.transform.translation

Improve type annotations for ament_mypy #5575

Improve type annotations for ament_mypy #5575

Uh oh!

Conversation

leander-dsouza commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Basic Info

Description of contribution in a few bullet points

Description of documentation updates required from your changes

Description of how this change was tested

Future work that may be required in bullet points

For Maintainers:

Uh oh!

codecov bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveMacenski commented Oct 2, 2025

Uh oh!

leander-dsouza commented Oct 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leander-dsouza commented Oct 6, 2025

Uh oh!

ajtudela commented Oct 6, 2025

Uh oh!

leander-dsouza commented Oct 6, 2025

Uh oh!

Uh oh!

ajtudela commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leander-dsouza commented Oct 2, 2025 •

edited

Loading

codecov bot commented Oct 2, 2025 •

edited

Loading