Type annotations do not reflect that Entity.get_attr()'s data_type parameter can be None

[christophebedard]

Generated by Generative AI

No response

Operating System:

all

ROS version or commit hash:

Rolling

RMW implementation (if applicable):

No response

RMW Configuration (if applicable):

No response

Client library (if applicable):

No response

'ros2 doctor --report' output

No response

Steps to reproduce issue

Run tests downstream of launch with the assumption that data_type can't be None, e.g., #887 (comment) for #887.

Expected behavior

Tests pass

Actual behavior

Tests fail if we assume that data_type can't be None, see these test failures for #887:

1. https://ci.ros2.org/job/ci_linux/24014/testReport/junit/test_launch_ros.test.test_launch_ros.frontend/test_component_container/test_launch_component_container_xml/
2. https://ci.ros2.org/job/ci_linux/24014/testReport/junit/test_launch_ros.test.test_launch_ros.frontend/test_node_frontend/test_launch_frontend_xml/

Additional information

See #887 (comment). The type annotation for data_type in launch_xml/launch_yaml's Entity.get_attr() is AllowedTypesType, which does not include None:

1. launch_xml:

   launch/launch_xml/launch_xml/entity.py

   Line 79 in 6854fae

   data_type: AllowedTypesType = str,

2. launch_yaml:

   launch/launch_yaml/launch_yaml/entity.py

   Line 102 in 6854fae

   data_type: AllowedTypesType = str,

However, the documentation in launch's Entity.get_attr() mentions that data_type can be None, in which case it behaves like str (IIUC), which is the default data_type value:

launch/launch/launch/frontend/entity.py

Line 89 in 6854fae

`data_type = None` will result in yaml parsing of the attribute value as a string.

Interestingly, launch's Entity.get_attr()'s type annotations are kind of incomplete, especially for data_type. They use TargetType, but that does not specify any type, and, critically, it does not specify None/Optional:

launch/launch/launch/frontend/entity.py

Lines 46 to 76 in 6854fae

	# We need a few overloads for type checking:
	# - Depending on optional, the return value is either T or Optional[T].
	# Unfortunately, if the optional is not present, we need another overload to denote the
	# default, so this has three values: true, false and not present.
	# - If no data type is passed, the return type is str. Similar to the above, it has two
	# possibilities: present or not present.
	# => 6 overloads to cover every combination
	@overload
	def get_attr(self, name: Text, *, data_type: Type[TargetType], # noqa: F811
	optional: bool, can_be_str: bool = True) -> Optional[TargetType]:
	...
	@overload
	def get_attr(self, name: Text, *, data_type: Type[TargetType], # noqa: F811
	can_be_str: bool = True) -> TargetType:
	...
	@overload
	def get_attr(self, name: Text, *, optional: bool, # noqa: F811
	can_be_str: bool = True) -> Optional[str]:
	...
	@overload
	def get_attr(self, name: Text, *, can_be_str: bool = True) -> str: # noqa: F811
	...
	def get_attr( # noqa: F811
	self,
	name,
	*,
	data_type=str,