Fix mypy and switch to precommit hooks

[mini-1235]

Based on my analysis in #5830 (comment), the base image currently used for linting was last updated ~6 months ago. This leads to inconsistent results between CI linting and running pre-commit locally. #5322, #5280, #5382, #5575 tried to fix mypy-related issues, but some of those fixes may be partially incorrect due to the outdated mypy version in CI.

This PR switches linting to use our own images to ensure they stay up to date and resolves #5830.

[mini-1235]

Sorry for spamming :(, I will fix this tomorrow

[tonynajjar]

@mini-1235 Thank you for taking this on, it's super annoying, I have to comment out ament_mypy to use pre-commit 😭

[SteveMacenski]

Alot of specific types were made into Any, I think we should be as specific as possible.

asserts should be removed from any runtime code (tests are fine). We don't want programs to terminate.

Also, check CI's output.

[mini-1235]

@SteveMacenski I have reverted some changes from my PR so you can take a look at the errors: https://github.com/ros-navigation/navigation2/actions/runs/20756574184/job/59600183826?pr=5837

Let me know how you would like me to proceed with these errors

[SteveMacenski]

It looks like we'd have to cast it in some way

footprint: list[Point32] = some_polygon.points 
point = cast(list[Point32], footprint)[index]
footprint = list(some_polygon.points)

Which ... works but also means that all ROS messages are broken (!).

I'd say disable it instead

[mypy]
# Disable union-attr and index errors for ROS message patterns
disable_error_code = index,union-attr

---

The other one is like:

nav2_simple_commander/nav2_simple_commander/robot_navigator.py:845:13: error: Returning Any from function declared to return "Path | None"  [no-any-return]

I don't see any solution to this other than having mypy stubs added for ROS message generation (Which should have been happening before?) -- so just # type: ignore[no-any-return] I guess.

---

This feels like a major break to ament_mypy / ROS messages with mypy. @tonynajjar do you see where / when this happened? It seems like this may warrant reverting if it just breaks the use of mypy for everything. We just are going to add exceptions/skip rules galore with defeats the purpose of trying. ROS 2 message need to have useful stubs.

Don't we have other python code interacting with ROS messages/services/actions -- why aren't those triggering issues like this parsing its internal parts / indexing vectors? Do they already have ignore lines or are other ROS messages correctly read and these are special fluke cases?

[mini-1235]

After taking a closer look at both ament_mypy and mypy, I would like to correct my earlier assumption:

• ament_mypy uses the system-installed mypy.
• The Ubuntu Noble mypy version (1.9.0) was last updated on Apr 15, 2024.
• Nav2 was already passing mypy checks with ament_mypy 0.19.1, see Introduction ament_mypy #4916, merged Apr 15, 2025
• Recent changelog for ament_mypy:

0.20.3 (2025-11-24)
Export typing information for ament linters (https://github.com/ament/ament_lint/issues/553)
Contributors: Michael Carlstrom
0.20.2 (2025-07-30)
fix setuptools deprecations (https://github.com/ament/ament_lint/issues/547)
Contributors: mosfet80
0.20.1 (2025-06-19)
0.20.0 (2025-04-24)
0.19.2 (2025-04-17)
0.19.1 (2025-02-02)
Add support for type stubs (https://github.com/ament/ament_lint/issues/516)
Contributors: Michael Carlstrom

• First fix for mypy: Fix ament mypy #5280 Jun 18, 2025 -> mypy 0.20.0

• Second fix: Fix ament mypy #5322 Jul 2, 2025 -> mypy 0.20.1

• Third fix: Added fixes for ament_mypy #5382 Jul 23, 2025 -> mypy 0.20.1

It seems like we end up needing to fix mypy after almost every rolling sync, and sometimes the newly introduced errors don't really make sense to me.

At this point, it seems more likely that the issue is caused by other packages in the base image instead of ament_mypy. The packages in the base image haven't been updated since July 23, 2025, which I believe is why we're no longer encountering the mypy issue.

I actually prepared this PR on Sunday night. When I tried to submit it on Monday, I started hitting different errors that I had to fix again(also today, when I tried to show you the errors), I honestly don't understand why 😮‍💨

[tonynajjar]

@tonynajjar do you see where / when this happened?

Sorry, no i don't know when it broke

[mini-1235]

Quoting from https://docs.astral.sh/ruff/faq/#how-does-ruff-compare-to-mypy-or-pyright-or-pyre:

Ruff is a linter, not a type checker. It can detect some of the same problems that a type checker can, but a type checker will catch certain errors that Ruff would miss. The opposite is also true: Ruff will catch certain errors that a type checker would typically ignore.

For example, unlike a type checker, Ruff will notify you if an import is unused, by looking for references to that import in the source code; on the other hand, a type checker could flag that you passed an integer argument to a function that expects a string, which Ruff would miss. The tools are complementary.

It's recommended that you use Ruff in conjunction with a type checker, like Mypy, Pyright, or Pyre, with Ruff providing faster feedback on lint violations and the type checker providing more detailed feedback on type errors.

Based on this, I don't think Ruff can replace mypy.

I also tested the other two type checkers mentioned in the docs:

pyright: https://gist.github.com/mini-1235/3b61429672788eecb0642607207af3bf
pyre: https://gist.github.com/mini-1235/7e5de2885b40b3c24472d2c0cf7496df

Neither of them looks better than mypy in our case

It seems likely that:
a. Type checking is still not fully supported in core packages? possibly related to ros2/rclpy#1257
or
b. There is an issue in upstream packages / nav2 that we haven't identified yet, leading to errors like:
nav2_simple_commander/nav2_simple_commander/robot_navigator.py:845:13: error: Returning Any from function declared to return "Path | None" [no-any-return]
which doesn't really make sense

Since the next rolling sync should be happening soon (starting tomorrow?), I guess we can wait a few more days to see whether the results change, and then decide whether it makes sense to keep or drop mypy checking in the codebase

[SteveMacenski]

I guess we can wait yes. I would love to have type checking properly in our CI, but if we're just going to mark everything as Any, I don't really see the point. This would probably good to file with somewhere that the mypy isn't accepting ROS interfaces correctly and describe the fashion of the problem (i.e. requiring casting now, doesn't ACK Result action types, etc) and that this is new and was previously working fine. Maybe someone on the inside of the python stack can tell us what happened and if there's a chance that can be changed back or what they recommend us to do.

Without some clarity / help as to what they think we should do, I think the only reasonable time efficient action for us is to remove mypy checks. We aren't a primarily python repository and there's only one demo-ware package that has run-time user exposed python, so its an unfortunate trade off of time-to-value.

[mini-1235]

This would probably good to file with somewhere that the mypy isn't accepting ROS interfaces correctly and describe the fashion of the problem (i.e. requiring casting now, doesn't ACK Result action types, etc) and that this is new and was previously working fine. Maybe someone on the inside of the python stack can tell us what happened and if there's a chance that can be changed back or what they recommend us to do.

Good idea, but I am not sure where this should be filed. The warnings come from mypy, but the root cause seems to originate from other packages, so I am not sure if it's appropriate to file this in the ament_lint repo. Any idea?

[SteveMacenski]

I'd file in rclpy and tag me with it. I think while its a linting issue, it affects linting in the rclpy as well.

... actually, how does rclpy's mypy config / codebase with linting look like? They use it, right? That should point us to a solution or do they really just have -> Any for everything? I assume their unit tests use geometry_msgs / std_msgs to see what they do with action results / services / topics type checking

[mini-1235]

... actually, how does rclpy's mypy config / codebase with linting look like? They use it, right?

No, I don't think so. I tried to run ament_mypy and I can see there are a lot of errors:

rclpy/test/test_lifecycle.py:185:42: error: Argument 1 to "call" of "Client" has incompatible type "GetState_Request"; expected "GetAvailableStates_Request"  [arg-type]
rclpy/test/test_lifecycle.py:187:47: error: "GetState_Response" has no attribute "available_states"  [attr-defined]
rclpy/test/test_lifecycle.py:192:11: error: Incompatible types in assignment (expression has type "GetAvailableTransitions_Request", variable has type "GetState_Request")  [assignment]
rclpy/test/test_lifecycle.py:193:12: error: Incompatible types in assignment (expression has type "GetAvailableTransitions_Response | None", variable has type "GetState_Response | None")  [assignment]
rclpy/test/test_lifecycle.py:193:47: error: Argument 1 to "call" of "Client" has incompatible type "GetState_Request"; expected "GetAvailableTransitions_Request"  [arg-type]
rclpy/test/test_lifecycle.py:196:63: error: "GetState_Response" has no attribute "available_transitions"  [attr-defined]
rclpy/test/test_lifecycle.py:198:11: error: Incompatible types in assignment (expression has type "GetAvailableTransitions_Request", variable has type "GetState_Request")  [assignment]
rclpy/test/test_lifecycle.py:199:12: error: Incompatible types in assignment (expression has type "GetAvailableTransitions_Response | None", variable has type "GetState_Response | None")  [assignment]
rclpy/test/test_lifecycle.py:199:42: error: Argument 1 to "call" of "Client" has incompatible type "GetState_Request"; expected "GetAvailableTransitions_Request"  [arg-type]
rclpy/test/test_lifecycle.py:202:63: error: "GetState_Response" has no attribute "available_transitions"  [attr-defined]
rclpy/test/test_executor.py: note: In member "test_create_task_coroutine_wake_from_another_thread" of class "TestExecutor":
rclpy/test/test_executor.py:361:28: error: "object" not callable  [operator]
rclpy/test/test_executor.py: note: In member "test_create_future_returns_future_with_executor_attached" of class "TestExecutor":
rclpy/test/test_executor.py:804:28: error: "object" not callable  [operator]
rclpy/test/test_events_executor.py: note: In member "__init__" of class "SubTestNode":
rclpy/test/test_events_executor.py:76:13: error: Argument 3 to "create_subscription" of "Node" has incompatible type "Callable[[BasicTypes], Coroutine[Any, Any, None]] | Callable[[BasicTypes], None]"; expected "Callable[[BasicTypes], None] | Callable[[BasicTypes, MessageInfo], None]"  [arg-type]
rclpy/test/test_events_executor.py: note: In member "__init__" of class "ServiceServerTestNode":
rclpy/test/test_events_executor.py:160:9: error: Cannot infer type argument 2 of "create_service" of "Node"  [misc]
rclpy/test/test_events_executor.py: note: In member "advance_feedback" of class "ActionServerTestNode":
rclpy/test/test_events_executor.py:340:44: error: Argument 1 to "publish_feedback" of "ServerGoalHandle" has incompatible type "Fibonacci_Feedback"; expected "FeedbackMessage[Fibonacci_Feedback]"  [arg-type]
rclpy/test/test_events_executor.py: note: In member "send_goal" of class "ActionClientTestNode":
rclpy/test/test_events_executor.py:393:31: error: Argument "feedback_callback" to "send_goal_async" of "ActionClient" has incompatible type "Callable[[Fibonacci_FeedbackMessage], None]"; expected "Callable[[Fibonacci_Feedback], None] | None"  [arg-type]
rclpy/test/test_events_executor.py: note: In member "_handle_goal_ack" of class "ActionClientTestNode":
rclpy/test/test_events_executor.py:402:41: error: Argument 1 to "add_done_callback" of "Future" has incompatible type "Callable[[Future[Fibonacci_GetResult_Response]], None]"; expected "Callable[[Future[GetResultServiceResponse[Fibonacci_Result]]], None]"  [arg-type]
rclpy/test/test_action_server.py: note: In member "feedback_callback" of class "MockActionClient":
rclpy/test/test_action_server.py:70:29: error: Incompatible types in assignment (expression has type "Fibonacci_FeedbackMessage", variable has type "None")  [assignment]
rclpy/test/test_action_server.py: note: In member "status_callback" of class "MockActionClient":
rclpy/test/test_action_server.py:73:27: error: Incompatible types in assignment (expression has type "GoalStatusArray", variable has type "None")  [assignment]
rclpy/test/test_action_server.py: note: In function "test_single_goal_accept":
rclpy/test/test_action_server.py:169:30: error: "CancelGoal_Request" has no attribute "order"  [attr-defined]
rclpy/test/test_action_server.py: note: In function "test_single_goal_reject":
rclpy/test/test_action_server.py:210:30: error: "CancelGoal_Request" has no attribute "order"  [attr-defined]
rclpy/test/test_action_server.py: note: In member "test_cancel_goal_accept" of class "TestActionServer":
rclpy/test/test_action_server.py:367:20: error: Value of type "Sequence[GoalInfo] | AbstractSet[GoalInfo]" is not indexable  [index]
rclpy/test/test_action_server.py: note: In member "test_cancel_goal_reject" of class "TestActionServer":
rclpy/test/test_action_server.py:386:25: error: Cannot infer type argument 2 of "ActionServer"  [misc]
rclpy/test/test_action_server.py: note: In function "test_feedback":
rclpy/test/test_action_server.py:709:42: error: Argument 1 to "publish_feedback" of "ServerGoalHandle" has incompatible type "Fibonacci_Feedback"; expected "FeedbackMessage[Any]"  [arg-type]
rclpy/test/test_action_server.py: note: In member "test_feedback" of class "TestActionServer":
rclpy/test/test_action_server.py:713:25: error: Cannot infer type argument 2 of "ActionServer"  [misc]
rclpy/test/test_action_server.py: note: In function "test_different_feedback_type_raises":
rclpy/test/test_action_server.py:742:46: error: Argument 1 to "publish_feedback" of "ServerGoalHandle" has incompatible type "Fibonacci_Feedback"; expected "FeedbackMessage[Any]"  [arg-type]
rclpy/test/test_action_server.py: note: In function "test_action_introspection_default_status":
rclpy/test/test_action_server.py:826:30: error: "CancelGoal_Request" has no attribute "order"  [attr-defined]
rclpy/test/test_action_server.py: note: In function "test_configure_introspection_content":
rclpy/test/test_action_server.py:885:30: error: "CancelGoal_Request" has no attribute "order"  [attr-defined]
rclpy/test/test_action_graph.py: note: In member "setUpClass" of class "TestActionGraph":
rclpy/test/test_action_graph.py:78:80: error: Argument 4 to "ActionServer" has incompatible type "Callable[[ServerGoalHandle[Fibonacci_Goal, Fibonacci_Result, Fibonacci_Feedback]], None]"; expected "Callable[[ServerGoalHandle[Fibonacci_Goal, Fibonacci_Result, Fibonacci_Feedback]], Fibonacci_Result] | None"  [arg-type]
rclpy/test/test_action_graph.py: note: In function "setUpClass":
rclpy/test/test_action_graph.py:78:90: error: Incompatible return value type (got "None", expected "Fibonacci_Result")  [return-value]
rclpy/test/test_action_graph.py: note: In member "setUpClass" of class "TestActionGraph":
rclpy/test/test_action_graph.py:81:80: error: Argument 4 to "ActionServer" has incompatible type "Callable[[ServerGoalHandle[Fibonacci_Goal, Fibonacci_Result, Fibonacci_Feedback]], None]"; expected "Callable[[ServerGoalHandle[Fibonacci_Goal, Fibonacci_Result, Fibonacci_Feedback]], Fibonacci_Result] | None"  [arg-type]
rclpy/test/test_action_graph.py: note: In function "setUpClass":
rclpy/test/test_action_graph.py:81:90: error: Incompatible return value type (got "None", expected "Fibonacci_Result")  [return-value]
rclpy/test/test_action_graph.py: note: In member "setUpClass" of class "TestActionGraph":
rclpy/test/test_action_graph.py:82:80: error: Argument 4 to "ActionServer" has incompatible type "Callable[[ServerGoalHandle[Fibonacci_Goal, Fibonacci_Result, Fibonacci_Feedback]], None]"; expected "Callable[[ServerGoalHandle[Fibonacci_Goal, Fibonacci_Result, Fibonacci_Feedback]], Fibonacci_Result] | None"  [arg-type]
rclpy/test/test_action_graph.py: note: In function "setUpClass":
rclpy/test/test_action_graph.py:82:90: error: Incompatible return value type (got "None", expected "Fibonacci_Result")  [return-value]
rclpy/test/test_action_client.py: note: In member "cancel_callback" of class "MockActionServer":
rclpy/test/test_action_client.py:73:9: error: Item "Sequence[GoalInfo]" of "Sequence[GoalInfo] | AbstractSet[GoalInfo]" has no attribute "append"  [union-attr]
rclpy/test/test_action_client.py:73:9: error: Item "AbstractSet[GoalInfo]" of "Sequence[GoalInfo] | AbstractSet[GoalInfo]" has no attribute "append"  [union-attr]
rclpy/test/test_action_client.py: note: In member "feedback_callback" of class "TestActionClient":
rclpy/test/test_action_client.py:112:25: error: Incompatible types in assignment (expression has type "Fibonacci_Feedback", variable has type "None")  [assignment]
rclpy/test/test_action_client.py: note: In member "test_send_cancel_async" of class "TestActionClient":
rclpy/test/test_action_client.py:372:17: error: Value of type "Sequence[GoalInfo] | AbstractSet[GoalInfo]" is not indexable  [index]
rclpy/test/test_action_client.py: note: In member "test_different_type_raises" of class "TestActionClient":
rclpy/test/test_action_client.py:400:30: error: Argument 1 to "send_goal" of "ActionClient" has incompatible type "str"; expected "Fibonacci_Goal"  [arg-type]
rclpy/test/test_action_client.py:400:30: note: Error code "arg-type" not covered by "type: ignore" comment
rclpy/test/test_action_client.py:402:36: error: Argument 1 to "send_goal_async" of "ActionClient" has incompatible type "str"; expected "Fibonacci_Goal"  [arg-type]
Found 291 errors in 29 files (checked 116 source files)

and ros2/rclpy#1257 is still open, so I believe that it's not fully supported yet

[SteveMacenski]

Then maybe ament_lint then. I'm just expecting less likelihood of a response from maintainers there. Maybe this is a sign we should also drop this if rclpy isn't even using type checking. If that's not, how could anything else be expected to.

[mini-1235]

Linking to: ros2/rosidl_python#228

[mini-1235]

From CI:

the "indexable" error is solved after the latest sync

[SteveMacenski]

Just append now it appears 👍 Once resolved we can remove the asserts and be done with this :-)

[mergify]

This pull request is in conflict. Could you fix it @mini-1235?

[tonynajjar]

Just append now it appears 👍 Once resolved we can remove the asserts and be done with this :-)

just to understand when to expect this, is this on us or waiting for something upstream?

[mini-1235]

The PMCs are currently working on this, see more in https://openrobotics.zulipchat.com/#narrow/channel/526027-ROS-General/topic/ROS.20Python.20Message.20Array.20Field.20Mutability

This will likely be resolved in the next rolling sync

[tonynajjar]

@SteveMacenski Can we comment out ament_mypy from the pre-commit config on main until it is fixed? It's making my life difficult, every time I want branch from main I need to comment it out myself and drag that uncommitted change around throughout all my git operations. I also tried to skip ament_mypy from the git hooks but our devcontainer setup brings it back when I rebuild....
But regardless of my struggle, it's simply a bug on main, we should find a short-term fix

[SteveMacenski]

@tonynajjar: Sure. Just comment on the ticket in parallel that it was disabled to be re-enabled later