From ab856cffae607b230f47870a3415ac0588a2ba6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= <101862279+haudren-woven@users.noreply.github.com> Date: Sat, 4 Mar 2023 03:47:48 +0900 Subject: [PATCH] Improve type checking (#679) (cherry picked from commit 06dc66b437f9908798a5da81dcec1b1646e94ee0) # Conflicts: # launch/launch/logging/__init__.py # launch/launch/substitutions/launch_log_dir.py # launch/launch/substitutions/python_expression.py # launch/launch/substitutions/this_launch_file.py # launch/launch/substitutions/this_launch_file_dir.py --- launch/doc/source/conf.py | 1 + .../examples/disable_emulate_tty_counters.py | 6 +- launch/examples/launch_counters.py | 6 +- launch/launch/__init__.py | 8 +-- launch/launch/action.py | 3 +- .../launch/actions/declare_launch_argument.py | 5 +- launch/launch/actions/execute_local.py | 54 +++++++++++------ launch/launch/actions/execute_process.py | 4 +- launch/launch/actions/opaque_coroutine.py | 19 +++--- .../launch/actions/register_event_handler.py | 5 +- launch/launch/actions/timer_action.py | 6 +- .../conditions/launch_configuration_equals.py | 3 + launch/launch/descriptions/executable.py | 13 ++-- launch/launch/event_handler.py | 13 ++-- .../event_handlers/on_action_event_base.py | 10 ++-- .../event_handlers/on_execution_complete.py | 10 ++-- .../launch/event_handlers/on_process_exit.py | 26 ++++---- launch/launch/event_handlers/on_process_io.py | 19 +++--- .../launch/event_handlers/on_process_start.py | 17 +++--- launch/launch/event_handlers/on_shutdown.py | 23 ++++--- .../events/process/process_targeted_event.py | 8 +-- .../launch/events/process/shutdown_process.py | 6 +- .../launch/events/process/signal_process.py | 4 +- launch/launch/frontend/entity.py | 60 +++++++++++++++---- launch/launch/frontend/expose.py | 6 +- launch/launch/frontend/parser.py | 28 ++++++--- launch/launch/launch_context.py | 3 +- launch/launch/launch_description_source.py | 2 +- .../any_launch_file_utilities.py | 6 +- .../frontend_launch_file_utilities.py | 4 +- .../python_launch_file_utilities.py | 2 + launch/launch/launch_introspector.py | 14 ++++- launch/launch/launch_service.py | 7 ++- launch/launch/logging/__init__.py | 13 +++- launch/launch/logging/handlers.py | 3 +- launch/launch/some_actions_type.py | 27 +++++---- launch/launch/some_entities_type.py | 31 ++++++++++ launch/launch/substitutions/anon_name.py | 14 ++--- .../substitutions/boolean_substitution.py | 26 ++++---- launch/launch/substitutions/command.py | 6 +- .../substitutions/environment_variable.py | 10 ++-- .../substitutions/equals_substitution.py | 21 +++++-- .../launch/substitutions/find_executable.py | 6 +- .../substitutions/launch_configuration.py | 8 +-- launch/launch/substitutions/launch_log_dir.py | 49 +++++++++++++++ .../substitutions/path_join_substitution.py | 4 +- .../launch/substitutions/python_expression.py | 33 +++++++++- .../launch/substitutions/this_launch_file.py | 7 ++- .../substitutions/this_launch_file_dir.py | 7 ++- launch/launch/utilities/__init__.py | 2 + .../normalize_to_list_of_entities_impl.py | 33 ++++++++++ launch/launch/utilities/signal_management.py | 10 ++-- launch/launch/utilities/type_utils.py | 16 +++-- launch/package.xml | 1 + launch/setup.cfg | 2 + .../launch/frontend/test_substitutions.py | 9 ++- launch/test/launch/test_event_handler.py | 4 +- .../utilities/test_signal_management.py | 6 +- .../test/launch/utilities/test_type_utils.py | 8 ++- launch/test/test_mypy.py | 23 +++++++ launch_testing/launch_testing/actions/test.py | 6 +- .../event_handlers/stdout_ready_listener.py | 6 +- 62 files changed, 552 insertions(+), 240 deletions(-) create mode 100644 launch/launch/some_entities_type.py create mode 100644 launch/launch/substitutions/launch_log_dir.py create mode 100644 launch/launch/utilities/normalize_to_list_of_entities_impl.py create mode 100644 launch/setup.cfg create mode 100644 launch/test/test_mypy.py diff --git a/launch/doc/source/conf.py b/launch/doc/source/conf.py index 5e039f0f6..8c318a78f 100644 --- a/launch/doc/source/conf.py +++ b/launch/doc/source/conf.py @@ -32,6 +32,7 @@ # -- Project information ----------------------------------------------------- +# type: ignore project = 'launch' copyright = '2018, Open Source Robotics Foundation, Inc.' # noqa diff --git a/launch/examples/disable_emulate_tty_counters.py b/launch/examples/disable_emulate_tty_counters.py index 231937ada..abf65d3db 100755 --- a/launch/examples/disable_emulate_tty_counters.py +++ b/launch/examples/disable_emulate_tty_counters.py @@ -24,7 +24,6 @@ import os import sys -from typing import cast sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) # noqa import launch # noqa: E402 @@ -37,10 +36,9 @@ def generate_launch_description(): ld.add_action(launch.actions.SetLaunchConfiguration('emulate_tty', 'false')) # Wire up stdout from processes - def on_output(event: launch.Event) -> None: + def on_output(event: launch.events.process.ProcessIO) -> None: for line in event.text.decode().splitlines(): - print('[{}] {}'.format( - cast(launch.events.process.ProcessIO, event).process_name, line)) + print('[{}] {}'.format(event.process_name, line)) ld.add_action(launch.actions.RegisterEventHandler(launch.event_handlers.OnProcessIO( on_stdout=on_output, diff --git a/launch/examples/launch_counters.py b/launch/examples/launch_counters.py index 8ffbb3982..e906f74e4 100755 --- a/launch/examples/launch_counters.py +++ b/launch/examples/launch_counters.py @@ -24,7 +24,6 @@ import os import platform import sys -from typing import cast sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) # noqa import launch # noqa: E402 @@ -55,10 +54,9 @@ def main(argv=sys.argv[1:]): # Setup a custom event handler for all stdout/stderr from processes. # Later, this will be a configurable, but always present, extension to the LaunchService. - def on_output(event: launch.Event) -> None: + def on_output(event: launch.events.process.ProcessIO) -> None: for line in event.text.decode().splitlines(): - print('[{}] {}'.format( - cast(launch.events.process.ProcessIO, event).process_name, line)) + print('[{}] {}'.format(event.process_name, line)) ld.add_action(launch.actions.RegisterEventHandler(launch.event_handlers.OnProcessIO( # this is the action ^ and this, the event handler ^ diff --git a/launch/launch/__init__.py b/launch/launch/__init__.py index a4c79e144..c5b506c55 100644 --- a/launch/launch/__init__.py +++ b/launch/launch/__init__.py @@ -32,8 +32,8 @@ from .launch_description_source import LaunchDescriptionSource from .launch_introspector import LaunchIntrospector from .launch_service import LaunchService -from .some_actions_type import SomeActionsType -from .some_actions_type import SomeActionsType_types_tuple +from .some_entities_type import SomeEntitiesType +from .some_entities_type import SomeEntitiesType_types_tuple from .some_substitutions_type import SomeSubstitutionsType from .some_substitutions_type import SomeSubstitutionsType_types_tuple from .substitution import Substitution @@ -57,8 +57,8 @@ 'LaunchDescriptionSource', 'LaunchIntrospector', 'LaunchService', - 'SomeActionsType', - 'SomeActionsType_types_tuple', + 'SomeEntitiesType', + 'SomeEntitiesType_types_tuple', 'SomeSubstitutionsType', 'SomeSubstitutionsType_types_tuple', 'Substitution', diff --git a/launch/launch/action.py b/launch/launch/action.py index fd7146576..4834680f9 100644 --- a/launch/launch/action.py +++ b/launch/launch/action.py @@ -14,6 +14,7 @@ """Module for Action class.""" +from typing import Dict from typing import Iterable from typing import List from typing import Optional @@ -62,7 +63,7 @@ def parse(entity: 'Entity', parser: 'Parser'): from .conditions import UnlessCondition if_cond = entity.get_attr('if', optional=True) unless_cond = entity.get_attr('unless', optional=True) - kwargs = {} + kwargs: Dict[str, Condition] = {} if if_cond is not None and unless_cond is not None: raise RuntimeError("if and unless conditions can't be used simultaneously") if if_cond is not None: diff --git a/launch/launch/actions/declare_launch_argument.py b/launch/launch/actions/declare_launch_argument.py index da1c63cd7..5290bc30c 100644 --- a/launch/launch/actions/declare_launch_argument.py +++ b/launch/launch/actions/declare_launch_argument.py @@ -14,7 +14,6 @@ """Module for the DeclareLaunchArgument action.""" -from typing import Iterable from typing import List from typing import Optional from typing import Text @@ -109,7 +108,7 @@ def __init__( *, default_value: Optional[SomeSubstitutionsType] = None, description: Optional[Text] = None, - choices: Iterable[Text] = None, + choices: List[Text] = None, **kwargs ) -> None: """Create a DeclareLaunchArgument action.""" @@ -196,7 +195,7 @@ def description(self) -> Text: return self.__description @property - def choices(self) -> List[Text]: + def choices(self) -> Optional[List[Text]]: """Getter for self.__choices.""" return self.__choices diff --git a/launch/launch/actions/execute_local.py b/launch/launch/actions/execute_local.py index 8fa0845b2..3062f868f 100644 --- a/launch/launch/actions/execute_local.py +++ b/launch/launch/actions/execute_local.py @@ -34,7 +34,7 @@ import launch.logging -from osrf_pycommon.process_utils import async_execute_process +from osrf_pycommon.process_utils import async_execute_process # type: ignore from osrf_pycommon.process_utils import AsyncSubprocessProtocol from .emit_event import EmitEvent @@ -63,7 +63,7 @@ from ..launch_context import LaunchContext from ..launch_description import LaunchDescription from ..launch_description_entity import LaunchDescriptionEntity -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType from ..some_substitutions_type import SomeSubstitutionsType from ..substitution import Substitution # noqa: F401 from ..substitutions import LaunchConfiguration @@ -97,8 +97,8 @@ def __init__( cached_output: bool = False, log_cmd: bool = False, on_exit: Optional[Union[ - SomeActionsType, - Callable[[ProcessExited, LaunchContext], Optional[SomeActionsType]] + SomeEntitiesType, + Callable[[ProcessExited, LaunchContext], Optional[SomeEntitiesType]] ]] = None, respawn: Union[bool, SomeSubstitutionsType] = False, respawn_delay: Optional[float] = None, @@ -193,9 +193,16 @@ def __init__( self.__sigterm_timeout = normalize_to_list_of_substitutions(sigterm_timeout) self.__sigkill_timeout = normalize_to_list_of_substitutions(sigkill_timeout) self.__emulate_tty = emulate_tty - self.__output = os.environ.get('OVERRIDE_LAUNCH_PROCESS_OUTPUT', output) - if not isinstance(self.__output, dict): - self.__output = normalize_to_list_of_substitutions(self.__output) + # Note: we need to use a temporary here so that we don't assign values with different types + # to the same variable + tmp_output: SomeSubstitutionsType = os.environ.get( + 'OVERRIDE_LAUNCH_PROCESS_OUTPUT', output + ) + self.__output: Union[dict, List[Substitution]] + if not isinstance(tmp_output, dict): + self.__output = normalize_to_list_of_substitutions(tmp_output) + else: + self.__output = tmp_output self.__output_format = output_format self.__log_cmd = log_cmd @@ -336,7 +343,7 @@ def __on_signal_process_event( def __on_process_stdin( self, event: ProcessIO - ) -> Optional[SomeActionsType]: + ) -> Optional[SomeEntitiesType]: self.__logger.warning( "in ExecuteProcess('{}').__on_process_stdin_event()".format(id(self)), ) @@ -345,7 +352,7 @@ def __on_process_stdin( def __on_process_output( self, event: ProcessIO, buffer: io.TextIOBase, logger: logging.Logger - ) -> Optional[SomeActionsType]: + ) -> None: to_write = event.text.decode(errors='replace') if buffer.closed: # buffer was probably closed by __flush_buffers on shutdown. Output without @@ -396,7 +403,7 @@ def __flush_buffers(self, event, context): def __on_process_output_cached( self, event: ProcessIO, buffer, logger - ) -> Optional[SomeActionsType]: + ) -> None: to_write = event.text.decode(errors='replace') last_cursor = buffer.tell() buffer.seek(0, os.SEEK_END) # go to end of buffer @@ -423,7 +430,7 @@ def __flush_cached_buffers(self, event, context): self.__output_format.format(line=line, this=self) ) - def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: + def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeEntitiesType]: due_to_sigint = cast(Shutdown, event).due_to_sigint return self._shutdown_process( context, @@ -578,9 +585,14 @@ async def __execute_process(self, context: LaunchContext) -> None: self.__logger.error("process has died [pid {}, exit code {}, cmd '{}'].".format( pid, returncode, ' '.join(filter(lambda part: part.strip(), cmd)) )) - await context.emit_event(ProcessExited(returncode=returncode, **process_event_args)) + await context.emit_event( + ProcessExited(returncode=returncode, **process_event_args) + ) # respawn the process if necessary - if not context.is_shutdown and not self.__shutdown_future.done() and self.__respawn: + if not context.is_shutdown\ + and self.__shutdown_future is not None\ + and not self.__shutdown_future.done()\ + and self.__respawn: if self.__respawn_delay is not None and self.__respawn_delay > 0.0: # wait for a timeout(`self.__respawn_delay`) to respawn the process # and handle shutdown event with future(`self.__shutdown_future`) @@ -608,7 +620,7 @@ def prepare(self, context: LaunchContext): # pid is added to the dictionary in the connection_made() method of the protocol. } - self.__respawn = perform_typed_substitution(context, self.__respawn, bool) + self.__respawn = cast(bool, perform_typed_substitution(context, self.__respawn, bool)) def execute(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEntity]]: """ @@ -663,7 +675,9 @@ def execute(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEnti ), OnProcessExit( target_action=self, - on_exit=self.__on_exit, + # TODO: This is also a little strange, OnProcessExit shouldn't ever be able to + # take a None for the callable, but this seems to be the default case? + on_exit=self.__on_exit, # type: ignore ), OnProcessExit( target_action=self, @@ -678,9 +692,13 @@ def execute(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEnti self.__shutdown_future = create_future(context.asyncio_loop) self.__logger = launch.logging.get_logger(name) if not isinstance(self.__output, dict): - self.__output = perform_substitutions(context, self.__output) - self.__stdout_logger, self.__stderr_logger = \ - launch.logging.get_output_loggers(name, self.__output) + self.__stdout_logger, self.__stderr_logger = \ + launch.logging.get_output_loggers( + name, perform_substitutions(context, self.__output) + ) + else: + self.__stdout_logger, self.__stderr_logger = \ + launch.logging.get_output_loggers(name, self.__output) context.asyncio_loop.create_task(self.__execute_process(context)) except Exception: for event_handler in event_handlers: diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 313d21e88..2db3c5993 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -29,6 +29,8 @@ from ..frontend import expose_action from ..frontend import Parser from ..some_substitutions_type import SomeSubstitutionsType + +from ..substitution import Substitution from ..substitutions import TextSubstitution _global_process_counter_lock = threading.Lock() @@ -254,7 +256,7 @@ def _parse_cmdline( :returns: a list of command line arguments. """ result_args = [] - arg = [] + arg: List[Substitution] = [] def _append_arg(): nonlocal arg diff --git a/launch/launch/actions/opaque_coroutine.py b/launch/launch/actions/opaque_coroutine.py index 0ebbd037a..3628075ef 100644 --- a/launch/launch/actions/opaque_coroutine.py +++ b/launch/launch/actions/opaque_coroutine.py @@ -17,7 +17,8 @@ import asyncio import collections.abc from typing import Any -from typing import Coroutine +from typing import Awaitable +from typing import Callable from typing import Dict from typing import Iterable from typing import List @@ -29,19 +30,19 @@ from ..event_handlers import OnShutdown from ..launch_context import LaunchContext from ..launch_description_entity import LaunchDescriptionEntity -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType from ..utilities import ensure_argument_type class OpaqueCoroutine(Action): """ - Action that adds a Python coroutine to the launch run loop. + Action that adds a Python coroutine function to the launch run loop. - The signature of a coroutine should be: + The signature of the coroutine function should be: .. code-block:: python - async def coroutine( + async def coroutine_func( context: LaunchContext, *args, **kwargs @@ -52,7 +53,7 @@ async def coroutine( .. code-block:: python - async def coroutine( + async def coroutine_func( *args, **kwargs ): @@ -63,7 +64,7 @@ async def coroutine( def __init__( self, *, - coroutine: Coroutine, + coroutine: Callable[..., Awaitable[None]], args: Optional[Iterable[Any]] = None, kwargs: Optional[Dict[Text, Any]] = None, ignore_context: bool = False, @@ -73,7 +74,7 @@ def __init__( super().__init__(**left_over_kwargs) if not asyncio.iscoroutinefunction(coroutine): raise TypeError( - "OpaqueCoroutine expected a coroutine for 'coroutine', got '{}'".format( + "OpaqueCoroutine expected a coroutine function for 'coroutine', got '{}'".format( type(coroutine) ) ) @@ -92,7 +93,7 @@ def __init__( self.__ignore_context = ignore_context # type: bool self.__future = None # type: Optional[asyncio.Future] - def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: + def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeEntitiesType]: """Cancel ongoing coroutine upon shutdown.""" if self.__future is not None: self.__future.cancel() diff --git a/launch/launch/actions/register_event_handler.py b/launch/launch/actions/register_event_handler.py index 85272fb75..54f8c15f2 100644 --- a/launch/launch/actions/register_event_handler.py +++ b/launch/launch/actions/register_event_handler.py @@ -23,6 +23,7 @@ from ..event_handler import BaseEventHandler from ..launch_context import LaunchContext from ..launch_description_entity import LaunchDescriptionEntity +from ..utilities import normalize_to_list_of_entities class RegisterEventHandler(Action): @@ -57,6 +58,8 @@ def describe_conditional_sub_entities(self) -> List[Tuple[ Iterable[LaunchDescriptionEntity], # list of conditional sub-entities ]]: event_handler_description = self.__event_handler.describe() + return [ - (event_handler_description[0], event_handler_description[1]) + (event_handler_description[0], + normalize_to_list_of_entities(event_handler_description[1])) ] if event_handler_description[1] else [] diff --git a/launch/launch/actions/timer_action.py b/launch/launch/actions/timer_action.py index 66d989e1f..7d470dcf8 100644 --- a/launch/launch/actions/timer_action.py +++ b/launch/launch/actions/timer_action.py @@ -40,7 +40,7 @@ from ..frontend import Parser from ..launch_context import LaunchContext from ..launch_description_entity import LaunchDescriptionEntity -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType from ..some_substitutions_type import SomeSubstitutionsType from ..some_substitutions_type import SomeSubstitutionsType_types_tuple from ..utilities import create_future @@ -137,7 +137,7 @@ def describe_conditional_sub_entities(self) -> List[Tuple[ """Return the actions that will result when the timer expires, but was not canceled.""" return [('{} seconds pass without being canceled'.format(self.__period), self.__actions)] - def handle(self, context: LaunchContext) -> Optional[SomeActionsType]: + def handle(self, context: LaunchContext) -> Optional[SomeEntitiesType]: """Handle firing of timer.""" context.extend_locals(self.__context_locals) return self.__actions @@ -157,7 +157,7 @@ def cancel(self) -> None: self._canceled_future.set_result(True) return None - def execute(self, context: LaunchContext) -> Optional[List['Action']]: + def execute(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEntity]]: """ Execute the action. diff --git a/launch/launch/conditions/launch_configuration_equals.py b/launch/launch/conditions/launch_configuration_equals.py index 11a35a798..d2140d5d0 100644 --- a/launch/launch/conditions/launch_configuration_equals.py +++ b/launch/launch/conditions/launch_configuration_equals.py @@ -14,12 +14,14 @@ """Module for LaunchConfigurationEquals class.""" +from typing import List from typing import Optional from typing import Text from ..condition import Condition from ..launch_context import LaunchContext from ..some_substitutions_type import SomeSubstitutionsType +from ..substitution import Substitution from ..utilities import normalize_to_list_of_substitutions from ..utilities import perform_substitutions @@ -44,6 +46,7 @@ def __init__( expected_value: Optional[SomeSubstitutionsType] ) -> None: self.__launch_configuration_name = launch_configuration_name + self.__expected_value: Optional[List[Substitution]] if expected_value is not None: self.__expected_value = normalize_to_list_of_substitutions(expected_value) else: diff --git a/launch/launch/descriptions/executable.py b/launch/launch/descriptions/executable.py index 109d1fc6d..6b1959c97 100644 --- a/launch/launch/descriptions/executable.py +++ b/launch/launch/descriptions/executable.py @@ -17,7 +17,6 @@ """Module for a description of an Executable.""" -import copy import os import re import shlex @@ -101,10 +100,10 @@ def __init__( normalize_to_list_of_substitutions(key), normalize_to_list_of_substitutions(value))) self.__arguments = arguments - self.__final_cmd = None - self.__final_cwd = None - self.__final_env = None - self.__final_name = None + self.__final_cmd: Optional[List[str]] = None + self.__final_cwd: Optional[str] = None + self.__final_env: Optional[Dict[str, str]] = None + self.__final_name: Optional[str] = None @property def name(self): @@ -178,7 +177,7 @@ def prepare(self, context: LaunchContext, action: Action): if self.__prefix_filter is not None: # no prefix given on construction prefix_filter = perform_substitutions(context, self.__prefix_filter) # Apply if filter regex matches (empty regex matches all strings) - should_apply_prefix = re.match(prefix_filter, os.path.basename(cmd[0])) + should_apply_prefix = re.match(prefix_filter, os.path.basename(cmd[0])) is not None if should_apply_prefix: cmd = shlex.split(perform_substitutions(context, self.__prefix)) + cmd self.__final_cmd = cmd @@ -198,7 +197,7 @@ def prepare(self, context: LaunchContext, action: Action): env[''.join([context.perform_substitution(x) for x in key])] = \ ''.join([context.perform_substitution(x) for x in value]) else: - env = copy.deepcopy(context.environment) + env = dict(context.environment) if self.__additional_env is not None: for key, value in self.__additional_env: env[''.join([context.perform_substitution(x) for x in key])] = \ diff --git a/launch/launch/event_handler.py b/launch/launch/event_handler.py index 16902671a..b2f9839a0 100644 --- a/launch/launch/event_handler.py +++ b/launch/launch/event_handler.py @@ -22,7 +22,7 @@ from typing import TYPE_CHECKING from .event import Event -from .some_actions_type import SomeActionsType +from .some_entities_type import SomeEntitiesType if TYPE_CHECKING: from .launch_context import LaunchContext # noqa: F401 @@ -77,7 +77,7 @@ def matches(self, event: Event) -> bool: """Return True if the given event should be handled by this event handler.""" return self.__matcher(event) - def describe(self) -> Tuple[Text, List[SomeActionsType]]: + def describe(self) -> Tuple[Text, List[SomeEntitiesType]]: """Return the description list with 0 as a string, and then LaunchDescriptionEntity's.""" return ( "{}(matcher='{}', handler='{}', handle_once={})".format( @@ -89,7 +89,7 @@ def describe(self) -> Tuple[Text, List[SomeActionsType]]: [] ) - def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeActionsType]: + def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeEntitiesType]: """ Handle the given event. @@ -99,6 +99,7 @@ def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeActions context.extend_locals({'event': event}) if self.handle_once: context.unregister_event_handler(self) + return None class EventHandler(BaseEventHandler): @@ -106,7 +107,7 @@ def __init__( self, *, matcher: Callable[[Event], bool], - entities: Optional[SomeActionsType] = None, + entities: Optional[SomeEntitiesType] = None, handle_once: bool = False ) -> None: """ @@ -128,14 +129,14 @@ def entities(self): """Getter for entities.""" return self.__entities - def describe(self) -> Tuple[Text, List[SomeActionsType]]: + def describe(self) -> Tuple[Text, List[SomeEntitiesType]]: """Return the description list with 0 as a string, and then LaunchDescriptionEntity's.""" text, actions = super().describe() if self.entities: actions.extend(self.entities) return (text, actions) - def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeActionsType]: + def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeEntitiesType]: """Handle the given event.""" super().handle(event, context) return self.entities diff --git a/launch/launch/event_handlers/on_action_event_base.py b/launch/launch/event_handlers/on_action_event_base.py index 4b7423e1f..6b3e096ca 100644 --- a/launch/launch/event_handlers/on_action_event_base.py +++ b/launch/launch/event_handlers/on_action_event_base.py @@ -28,7 +28,7 @@ from ..event_handler import BaseEventHandler from ..launch_context import LaunchContext from ..launch_description_entity import LaunchDescriptionEntity -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType if TYPE_CHECKING: from ..action import Action # noqa: F401 @@ -42,8 +42,8 @@ def __init__( *, action_matcher: Optional[Union[Callable[['Action'], bool], 'Action']], on_event: Union[ - SomeActionsType, - Callable[[Event, LaunchContext], Optional[SomeActionsType]] + SomeEntitiesType, + Callable[[Event, LaunchContext], Optional[SomeEntitiesType]] ], target_event_cls: Type[Event], target_action_cls: Type['Action'], @@ -103,7 +103,7 @@ def event_matcher(event): else: self.__actions_on_event = [on_event] - def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: + def handle(self, event: Event, context: LaunchContext) -> Optional[SomeEntitiesType]: """Handle the given event.""" super().handle(event, context) @@ -111,7 +111,7 @@ def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsTy return self.__actions_on_event return self.__on_event(event, context) - def describe(self) -> Tuple[Text, List[SomeActionsType]]: + def describe(self) -> Tuple[Text, List[SomeEntitiesType]]: """Return a description tuple.""" text, actions = super().describe() if self.__actions_on_event: diff --git a/launch/launch/event_handlers/on_execution_complete.py b/launch/launch/event_handlers/on_execution_complete.py index 4fa19cd59..7f76d2c01 100644 --- a/launch/launch/event_handlers/on_execution_complete.py +++ b/launch/launch/event_handlers/on_execution_complete.py @@ -22,7 +22,7 @@ from ..event import Event from ..events import ExecutionComplete from ..launch_context import LaunchContext -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType if TYPE_CHECKING: from .. import Action # noqa: F401 @@ -43,16 +43,16 @@ def __init__( Optional[Union[Callable[['Action'], bool], 'Action']] = None, on_completion: Union[ - SomeActionsType, - Callable[[ExecutionComplete, LaunchContext], Optional[SomeActionsType]]], + SomeEntitiesType, + Callable[[ExecutionComplete, LaunchContext], Optional[SomeEntitiesType]]], **kwargs ) -> None: """Create an OnExecutionComplete event handler.""" from ..action import Action # noqa: F811 on_completion = cast( Union[ - SomeActionsType, - Callable[[Event, LaunchContext], Optional[SomeActionsType]]], + SomeEntitiesType, + Callable[[Event, LaunchContext], Optional[SomeEntitiesType]]], on_completion) super().__init__( action_matcher=target_action, diff --git a/launch/launch/event_handlers/on_process_exit.py b/launch/launch/event_handlers/on_process_exit.py index c037e21a4..41efb2bf9 100644 --- a/launch/launch/event_handlers/on_process_exit.py +++ b/launch/launch/event_handlers/on_process_exit.py @@ -24,11 +24,11 @@ from ..event import Event from ..events.process import ProcessExited from ..launch_context import LaunchContext -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType if TYPE_CHECKING: - from ..actions import Action # noqa: F401 + from ..action import Action # noqa: F401 from ..actions import ExecuteLocal # noqa: F401 @@ -47,24 +47,22 @@ def __init__( Optional[Union[Callable[['ExecuteLocal'], bool], 'ExecuteLocal']] = None, on_exit: Union[ - SomeActionsType, - Callable[[ProcessExited, LaunchContext], Optional[SomeActionsType]] + SomeEntitiesType, + Callable[[ProcessExited, LaunchContext], Optional[SomeEntitiesType]] ], **kwargs ) -> None: """Create an OnProcessExit event handler.""" from ..actions import ExecuteLocal # noqa: F811 - target_action = cast( - Optional[Union[Callable[['Action'], bool], 'Action']], - target_action) - on_exit = cast( - Union[ - SomeActionsType, - Callable[[Event, LaunchContext], Optional[SomeActionsType]]], - on_exit) super().__init__( - action_matcher=target_action, - on_event=on_exit, + action_matcher=cast( + Optional[Union[Callable[['Action'], bool], 'Action']], target_action + ), + on_event=cast( + Union[SomeEntitiesType, + Callable[[Event, LaunchContext], Optional[SomeEntitiesType]]], + on_exit + ), target_event_cls=ProcessExited, target_action_cls=ExecuteLocal, **kwargs, diff --git a/launch/launch/event_handlers/on_process_io.py b/launch/launch/event_handlers/on_process_io.py index d5b19f6a4..66d79dc4b 100644 --- a/launch/launch/event_handlers/on_process_io.py +++ b/launch/launch/event_handlers/on_process_io.py @@ -24,10 +24,10 @@ from ..event import Event from ..events.process import ProcessIO from ..launch_context import LaunchContext -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType if TYPE_CHECKING: - from ..actions import Action # noqa: F401 + from ..action import Action # noqa: F401 from ..actions import ExecuteLocal # noqa: F401 @@ -35,24 +35,21 @@ class OnProcessIO(OnActionEventBase): """Convenience class for handling I/O from processes via events.""" # TODO(wjwwood): make the __init__ more flexible like OnProcessExit, so - # that it can take SomeActionsType directly or a callable that returns it. + # that it can take SomeEntitiesType directly or a callable that returns it. def __init__( self, *, target_action: - Optional[Union[Callable[['ExecuteLocal'], bool], 'ExecuteLocal']] = None, - on_stdin: Callable[[ProcessIO], Optional[SomeActionsType]] = None, - on_stdout: Callable[[ProcessIO], Optional[SomeActionsType]] = None, - on_stderr: Callable[[ProcessIO], Optional[SomeActionsType]] = None, + Optional[Union[Callable[['Action'], bool], 'Action']] = None, + on_stdin: Callable[[ProcessIO], Optional[SomeEntitiesType]] = None, + on_stdout: Callable[[ProcessIO], Optional[SomeEntitiesType]] = None, + on_stderr: Callable[[ProcessIO], Optional[SomeEntitiesType]] = None, **kwargs ) -> None: """Create an OnProcessIO event handler.""" from ..actions import ExecuteLocal # noqa: F811 - target_action = cast( - Optional[Union[Callable[['Action'], bool], 'Action']], - target_action) - def handle(event: Event, _: LaunchContext) -> Optional[SomeActionsType]: + def handle(event: Event, _: LaunchContext) -> Optional[SomeEntitiesType]: event = cast(ProcessIO, event) if event.from_stdout and on_stdout is not None: return on_stdout(event) diff --git a/launch/launch/event_handlers/on_process_start.py b/launch/launch/event_handlers/on_process_start.py index 158f4a974..0c1f2304d 100644 --- a/launch/launch/event_handlers/on_process_start.py +++ b/launch/launch/event_handlers/on_process_start.py @@ -24,10 +24,10 @@ from ..event import Event from ..events.process import ProcessStarted from ..launch_context import LaunchContext -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType if TYPE_CHECKING: - from ..actions import Action # noqa: F401 + from ..action import Action # noqa: F401 from ..actions import ExecuteProcess # noqa: F401 @@ -43,22 +43,19 @@ def __init__( self, *, target_action: - Optional[Union[Callable[['ExecuteProcess'], bool], 'ExecuteProcess']] = None, + Optional[Union[Callable[['Action'], bool], 'Action']] = None, on_start: Union[ - SomeActionsType, - Callable[[ProcessStarted, LaunchContext], Optional[SomeActionsType]]], + SomeEntitiesType, + Callable[[ProcessStarted, LaunchContext], Optional[SomeEntitiesType]]], **kwargs ) -> None: """Create an OnProcessStart event handler.""" from ..actions import ExecuteProcess # noqa: F811 - target_action = cast( - Optional[Union[Callable[['Action'], bool], 'Action']], - target_action) on_start = cast( Union[ - SomeActionsType, - Callable[[Event, LaunchContext], Optional[SomeActionsType]]], + SomeEntitiesType, + Callable[[Event, LaunchContext], Optional[SomeEntitiesType]]], on_start) super().__init__( action_matcher=target_action, diff --git a/launch/launch/event_handlers/on_shutdown.py b/launch/launch/event_handlers/on_shutdown.py index 645f19818..c5ad1928c 100644 --- a/launch/launch/event_handlers/on_shutdown.py +++ b/launch/launch/event_handlers/on_shutdown.py @@ -24,21 +24,29 @@ from ..event import Event from ..event_handler import BaseEventHandler from ..events import Shutdown -from ..some_actions_type import SomeActionsType +from ..some_entities_type import SomeEntitiesType from ..utilities import is_a_subclass if TYPE_CHECKING: from ..launch_context import LaunchContext # noqa: F401 +def gen_handler( + entities: SomeEntitiesType + ) -> Callable[[Shutdown, 'LaunchContext'], SomeEntitiesType]: + def handler(event: Shutdown, context: 'LaunchContext') -> SomeEntitiesType: + return entities + return handler + + class OnShutdown(BaseEventHandler): """Convenience class for handling the launch shutdown event.""" def __init__( self, *, - on_shutdown: Union[SomeActionsType, - Callable[[Shutdown, 'LaunchContext'], Optional[SomeActionsType]]], + on_shutdown: Union[SomeEntitiesType, + Callable[[Shutdown, 'LaunchContext'], Optional[SomeEntitiesType]]], **kwargs ) -> None: """Create an OnShutdown event handler.""" @@ -48,11 +56,12 @@ def __init__( ) # TODO(wjwwood) check that it is not only callable, but also a callable that matches # the correct signature for a handler in this case - self.__on_shutdown = on_shutdown - if not callable(on_shutdown): - self.__on_shutdown = (lambda event, context: on_shutdown) + if callable(on_shutdown): + self.__on_shutdown = on_shutdown + else: + self.__on_shutdown = gen_handler(on_shutdown) - def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeActionsType]: + def handle(self, event: Event, context: 'LaunchContext') -> Optional[SomeEntitiesType]: """Handle the given event.""" super().handle(event, context) return self.__on_shutdown(cast(Shutdown, event), context) diff --git a/launch/launch/events/process/process_targeted_event.py b/launch/launch/events/process/process_targeted_event.py index 229fab5b3..0ef27ba6d 100644 --- a/launch/launch/events/process/process_targeted_event.py +++ b/launch/launch/events/process/process_targeted_event.py @@ -20,7 +20,7 @@ from ...event import Event if TYPE_CHECKING: - from ...actions import ExecuteProcess # noqa: F401 + from ...actions import ExecuteLocal # noqa: F401 class ProcessTargetedEvent(Event): @@ -28,7 +28,7 @@ class ProcessTargetedEvent(Event): name = 'launch.events.process.ProcessTargetedEvent' - def __init__(self, *, process_matcher: Callable[['ExecuteProcess'], bool]) -> None: + def __init__(self, *, process_matcher: Callable[['ExecuteLocal'], bool]) -> None: """ Create a ProcessTargetedEvent. @@ -40,12 +40,12 @@ def __init__(self, *, process_matcher: Callable[['ExecuteProcess'], bool]) -> No - :func:`launch.events.process.matches_executable()` :param: process_matcher is a predicate which can determine if an - ExecuteProcess action matches this event or not + ExecuteLocal action matches this event or not """ super().__init__() self.__process_matcher = process_matcher @property - def process_matcher(self) -> Callable[['ExecuteProcess'], bool]: + def process_matcher(self) -> Callable[['ExecuteLocal'], bool]: """Getter for process_matcher.""" return self.__process_matcher diff --git a/launch/launch/events/process/shutdown_process.py b/launch/launch/events/process/shutdown_process.py index e205e8860..fe25ff54b 100644 --- a/launch/launch/events/process/shutdown_process.py +++ b/launch/launch/events/process/shutdown_process.py @@ -20,14 +20,14 @@ from .process_targeted_event import ProcessTargetedEvent if TYPE_CHECKING: - from ...actions import ExecuteProcess # noqa: F401 + from ...actions import ExecuteLocal # noqa: F401 class ShutdownProcess(ProcessTargetedEvent): """ Event emitted when a process should begin shutting down. - This event is handled by the launch.actions.ExecuteProcess action, see it + This event is handled by the launch.actions.ExecuteLocal action, see it for details on what happens when this is emitted. Also see ProcessTargetedEvent for details on how to target a specific @@ -36,6 +36,6 @@ class ShutdownProcess(ProcessTargetedEvent): name = 'launch.events.process.ShutdownProcess' - def __init__(self, *, process_matcher: Callable[['ExecuteProcess'], bool]) -> None: + def __init__(self, *, process_matcher: Callable[['ExecuteLocal'], bool]) -> None: """Create a ShutdownProcess event.""" super().__init__(process_matcher=process_matcher) diff --git a/launch/launch/events/process/signal_process.py b/launch/launch/events/process/signal_process.py index 6bd42f52a..75ad25c47 100644 --- a/launch/launch/events/process/signal_process.py +++ b/launch/launch/events/process/signal_process.py @@ -24,7 +24,7 @@ from ...utilities import ensure_argument_type if TYPE_CHECKING: - from ...actions import ExecuteProcess # noqa: F401 + from ...actions import ExecuteLocal # noqa: F401 class SignalProcess(ProcessTargetedEvent): @@ -35,7 +35,7 @@ class SignalProcess(ProcessTargetedEvent): def __init__( self, *, signal_number: Union[Text, signal_module.Signals], - process_matcher: Callable[['ExecuteProcess'], bool] + process_matcher: Callable[['ExecuteLocal'], bool] ) -> None: """ Create a SignalProcess event. diff --git a/launch/launch/frontend/entity.py b/launch/launch/frontend/entity.py index 53d417850..3816e6115 100644 --- a/launch/launch/frontend/entity.py +++ b/launch/launch/frontend/entity.py @@ -15,12 +15,15 @@ """Module for Entity class.""" from typing import List +from typing import Literal from typing import Optional +from typing import overload from typing import Text -from typing import Union +from typing import Type +from typing import TypeVar -from launch.utilities.type_utils import AllowedTypesType -from launch.utilities.type_utils import AllowedValueType + +TargetType = TypeVar('TargetType') class Entity: @@ -41,17 +44,50 @@ def children(self) -> List['Entity']: """Get the Entity's children.""" raise NotImplementedError() - def get_attr( + # 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], + optional: Literal[False], can_be_str: bool = True) -> TargetType: + ... + + @overload + def get_attr(self, name: Text, *, data_type: Type[TargetType], # noqa: F811 + optional: Literal[True], 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: Literal[False], # noqa: F811 + can_be_str: bool = True) -> str: + ... + + @overload + def get_attr(self, name: Text, *, optional: Literal[True], # 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: Text, + name, *, - data_type: AllowedTypesType = str, - optional: bool = False, - can_be_str: bool = True, - ) -> Optional[Union[ - AllowedValueType, - List['Entity'], - ]]: + data_type=str, + optional=False, + can_be_str=True, + ): """ Access an attribute of the entity. diff --git a/launch/launch/frontend/expose.py b/launch/launch/frontend/expose.py index dd94596a0..587f1568c 100644 --- a/launch/launch/frontend/expose.py +++ b/launch/launch/frontend/expose.py @@ -16,6 +16,8 @@ import functools import inspect +from typing import Any +from typing import Dict from typing import Iterable from typing import Optional from typing import Text @@ -28,8 +30,8 @@ from .entity import Entity # noqa: F401 from .parser import Parser # noqa: F401 -action_parse_methods = {} -substitution_parse_methods = {} +action_parse_methods: Dict[str, Any] = {} +substitution_parse_methods: Dict[str, Any] = {} def instantiate_action(entity: 'Entity', parser: 'Parser') -> Action: diff --git a/launch/launch/frontend/parser.py b/launch/launch/frontend/parser.py index 2bfea7d98..50e6e860e 100644 --- a/launch/launch/frontend/parser.py +++ b/launch/launch/frontend/parser.py @@ -17,22 +17,19 @@ import itertools import os.path +import sys import traceback from typing import List from typing import Optional from typing import Set from typing import Text from typing import TextIO +from typing import Tuple from typing import Type from typing import TYPE_CHECKING from typing import Union import warnings -try: - import importlib.metadata as importlib_metadata -except ModuleNotFoundError: - import importlib_metadata - from .entity import Entity from .expose import instantiate_action from .parse_substitution import parse_if_substitutions @@ -45,6 +42,11 @@ from ..utilities.type_utils import StrSomeValueType from ..utilities.typing_file_path import FilePath +if sys.version_info >= (3, 8): + import importlib.metadata as importlib_metadata +else: + import importlib_metadata + if TYPE_CHECKING: from ..launch_description import LaunchDescription @@ -137,6 +139,7 @@ def parse_description(self, entity: Entity) -> 'LaunchDescription': def get_available_extensions(cls) -> List[Text]: """Return the registered extensions.""" cls.load_parser_implementations() + assert(cls.frontend_parsers is not None) return cls.frontend_parsers.keys() @classmethod @@ -148,6 +151,7 @@ def is_extension_valid( warnings.warn( 'Parser.is_extension_valid is deprecated, use Parser.is_filename_valid instead') cls.load_parser_implementations() + assert(cls.frontend_parsers is not None) return extension in cls.frontend_parsers @classmethod @@ -160,6 +164,7 @@ def get_parser_from_extension( 'Parser.get_parser_from_extension is deprecated, ' 'use Parser.get_parsers_from_filename instead') cls.load_parser_implementations() + assert(cls.frontend_parsers is not None) try: return cls.frontend_parsers[extension] except KeyError: @@ -180,6 +185,7 @@ def is_filename_valid( ) -> bool: """Return `True` if the filename is valid for any parser.""" cls.load_parser_implementations() + assert(cls.frontend_parsers is not None) return any( parser.may_parse(filename) for parser in cls.frontend_parsers.values() @@ -192,6 +198,7 @@ def get_parsers_from_filename( ) -> List[Type['Parser']]: """Return a list of parsers which entity loaded with a markup file.""" cls.load_parser_implementations() + assert(cls.frontend_parsers is not None) return [ parser for parser in cls.frontend_parsers.values() if parser.may_parse(filename) @@ -201,6 +208,7 @@ def get_parsers_from_filename( def get_file_extensions_from_parsers(cls) -> Set[Type['Parser']]: """Return a set of file extensions known to the parser implementations.""" cls.load_parser_implementations() + assert(cls.frontend_parsers is not None) return set(itertools.chain.from_iterable( parser_extension.get_file_extensions() for parser_extension in cls.frontend_parsers.values() @@ -210,7 +218,7 @@ def get_file_extensions_from_parsers(cls) -> Set[Type['Parser']]: def load( cls, file: Union[FilePath, TextIO], - ) -> (Entity, 'Parser'): + ) -> Tuple[Entity, 'Parser']: """ Parse an Entity from a markup language-based launch file. @@ -222,16 +230,18 @@ def load( # Imported here, to avoid recursive import. cls.load_parser_implementations() - try: + fileobj: TextIO + if isinstance(file, (str, bytes, os.PathLike)): fileobj = open(file, 'r') didopen = True - except TypeError: + else: fileobj = file didopen = False try: filename = getattr(fileobj, 'name', '') implementations = cls.get_parsers_from_filename(filename) + assert(cls.frontend_parsers is not None) implementations += [ parser for parser in cls.frontend_parsers.values() if parser not in implementations @@ -255,4 +265,4 @@ def load( @classmethod def get_file_extensions(cls) -> Set[Text]: """Return the set of file extensions known to this parser.""" - return {} + return set() diff --git a/launch/launch/launch_context.py b/launch/launch/launch_context.py index 498b8a7de..e86092f4f 100644 --- a/launch/launch/launch_context.py +++ b/launch/launch/launch_context.py @@ -22,6 +22,7 @@ from typing import Iterable from typing import List # noqa: F401 from typing import Mapping +from typing import MutableMapping from typing import Optional from typing import Text @@ -201,7 +202,7 @@ def launch_configurations(self) -> Dict[Text, Text]: return self.__launch_configurations @property - def environment(self) -> Mapping[Text, Text]: + def environment(self) -> MutableMapping[Text, Text]: """Getter for environment variables dictionary.""" return os.environ diff --git a/launch/launch/launch_description_source.py b/launch/launch/launch_description_source.py index a4d23ee56..04f7315c9 100644 --- a/launch/launch/launch_description_source.py +++ b/launch/launch/launch_description_source.py @@ -49,7 +49,7 @@ def __init__( """ self.__launch_description: Optional[LaunchDescription] = launch_description self.__expanded_location: Optional[Text] = None - self.__location: SomeSubstitutionsType = normalize_to_list_of_substitutions(location) + self.__location = normalize_to_list_of_substitutions(location) self.__method: str = method self.__logger = launch.logging.get_logger(__name__) diff --git a/launch/launch/launch_description_sources/any_launch_file_utilities.py b/launch/launch/launch_description_sources/any_launch_file_utilities.py index 37803292d..b6a81d5be 100644 --- a/launch/launch/launch_description_sources/any_launch_file_utilities.py +++ b/launch/launch/launch_description_sources/any_launch_file_utilities.py @@ -15,6 +15,8 @@ """Python package utility functions related to loading Frontend Launch Files.""" import os +from typing import Callable +from typing import List from typing import Text from typing import Type @@ -38,7 +40,9 @@ def get_launch_description_from_any_launch_file( :raise `SyntaxError`: Invalid file. The file may have a syntax error in it. :raise `ValueError`: Invalid file. The file may not be a text file. """ - loaders = [get_launch_description_from_frontend_launch_file] + loaders: List[Callable[[str], LaunchDescription]] =\ + [get_launch_description_from_frontend_launch_file] + launch_file_name = os.path.basename(launch_file_path) extension = os.path.splitext(launch_file_name)[1] if extension: diff --git a/launch/launch/launch_description_sources/frontend_launch_file_utilities.py b/launch/launch/launch_description_sources/frontend_launch_file_utilities.py index e774e1f84..4209801a6 100644 --- a/launch/launch/launch_description_sources/frontend_launch_file_utilities.py +++ b/launch/launch/launch_description_sources/frontend_launch_file_utilities.py @@ -31,5 +31,5 @@ def get_launch_description_from_frontend_launch_file( parser: Type[Parser] = Parser ) -> LaunchDescription: """Load a `LaunchDescription` from a declarative (markup based) launch file.""" - root_entity, parser = parser.load(frontend_launch_file_path) - return parser.parse_description(root_entity) + root_entity, parser_instance = parser.load(frontend_launch_file_path) + return parser_instance.parse_description(root_entity) diff --git a/launch/launch/launch_description_sources/python_launch_file_utilities.py b/launch/launch/launch_description_sources/python_launch_file_utilities.py index e5715c59b..87011c7f9 100644 --- a/launch/launch/launch_description_sources/python_launch_file_utilities.py +++ b/launch/launch/launch_description_sources/python_launch_file_utilities.py @@ -33,6 +33,8 @@ def load_python_launch_file_as_module(python_launch_file_path: Text) -> ModuleTy """Load a given Python launch file (by path) as a Python module.""" loader = SourceFileLoader('python_launch_file', python_launch_file_path) spec = spec_from_loader(loader.name, loader) + if spec is None: + raise RuntimeError('Failed to load module spec!') mod = module_from_spec(spec) loader.exec_module(mod) return mod diff --git a/launch/launch/launch_introspector.py b/launch/launch/launch_introspector.py index 41cb34ca8..1abcbcef4 100644 --- a/launch/launch/launch_introspector.py +++ b/launch/launch/launch_introspector.py @@ -15,6 +15,7 @@ """Module for the LaunchIntrospector class.""" from typing import cast +from typing import Iterable from typing import List from typing import Text @@ -63,7 +64,7 @@ def tree_like_indent(lines: List[Text]) -> List[Text]: return result -def format_entities(entities: List[LaunchDescriptionEntity]) -> List[Text]: +def format_entities(entities: Iterable[LaunchDescriptionEntity]) -> List[Text]: """Return a list of lines of text that represent of a list of LaunchDescriptionEntity's.""" result = [] for entity in entities: @@ -84,9 +85,16 @@ def format_event_handler(event_handler: BaseEventHandler) -> List[Text]: """Return a text representation of an event handler.""" if hasattr(event_handler, 'describe'): # TODO(wjwwood): consider supporting mode complex descriptions of branching - description, entities = event_handler.describe() # type: ignore + description, entities = event_handler.describe() result = [description] - result.extend(indent(format_entities(entities))) + if isinstance(entities, LaunchDescriptionEntity): + result.extend(indent(format_entities([entities]))) + else: + # Note due to a bug in mypy ( https://github.com/python/mypy/issues/13709 ), + # the variable is not correctly narrowed to Iterable[...] in this branch... + result.extend( + indent(format_entities(cast(Iterable[LaunchDescriptionEntity], entities))) + ) return result else: return ["EventHandler('{}')".format(hex(id(event_handler)))] diff --git a/launch/launch/launch_service.py b/launch/launch/launch_service.py index 3dd1e7429..0c7e1827b 100644 --- a/launch/launch/launch_service.py +++ b/launch/launch/launch_service.py @@ -32,7 +32,7 @@ import launch.logging -import osrf_pycommon.process_utils +import osrf_pycommon.process_utils # type: ignore from .event import Event from .event_handlers import OnIncludeLaunchDescription @@ -42,7 +42,7 @@ from .launch_context import LaunchContext from .launch_description import LaunchDescription from .launch_description_entity import LaunchDescriptionEntity -from .some_actions_type import SomeActionsType +from .some_entities_type import SomeEntitiesType from .utilities import AsyncSafeSignalManager from .utilities import visit_all_entities_and_collect_futures @@ -376,7 +376,7 @@ def run(self, *, shutdown_when_idle=True) -> int: except KeyboardInterrupt: continue - def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: + def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeEntitiesType]: self.__shutting_down = True self.__context._set_is_shutdown(True) return None @@ -423,6 +423,7 @@ def shutdown(self, force_sync=False) -> Optional[Coroutine]: reason='LaunchService.shutdown() called', due_to_sigint=False, force_sync=force_sync ) + return None @property def context(self): diff --git a/launch/launch/logging/__init__.py b/launch/launch/logging/__init__.py index 9f9be2e65..653064021 100644 --- a/launch/launch/logging/__init__.py +++ b/launch/launch/logging/__init__.py @@ -24,7 +24,11 @@ import socket import sys +<<<<<<< HEAD from typing import Iterable +======= +from typing import Any +>>>>>>> 06dc66b (Improve type checking (#679)) from typing import List from . import handlers @@ -293,7 +297,7 @@ def log_launch_config(*, logger=logging.root): ))) -def get_logger(name=None): +def get_logger(name=None) -> logging.Logger: """Get named logger, configured to output to screen and launch main log file.""" logger = logging.getLogger(name) screen_handler = launch_config.get_screen_handler() @@ -468,8 +472,13 @@ def get_output_loggers(process_name, output_config): ) +# Mypy does not support dynamic base classes, so workaround by typing the base +# class as Any +_Base = logging.getLoggerClass() # type: Any + + # Track all loggers to support module resets -class LaunchLogger(logging.getLoggerClass()): +class LaunchLogger(_Base): all_loggers: List[logging.Logger] = [] def __new__(cls, *args, **kwargs): diff --git a/launch/launch/logging/handlers.py b/launch/launch/logging/handlers.py index 454f296a1..a172c53c0 100644 --- a/launch/launch/logging/handlers.py +++ b/launch/launch/logging/handlers.py @@ -15,6 +15,7 @@ """Module with handlers for launch specific logging.""" import sys +import types def with_per_logger_formatting(cls): @@ -50,7 +51,7 @@ def format(self, record): # noqa # TODO(hidmic): replace module wrapper with module-level __getattr__ # implementation when we switch to Python 3.7+ -class _module_wrapper: +class _module_wrapper(types.ModuleType): """Provide all Python `logging` module handlers with per logger formatting support.""" def __init__(self, wrapped_module): diff --git a/launch/launch/some_actions_type.py b/launch/launch/some_actions_type.py index 03d819584..c5c4030c8 100644 --- a/launch/launch/some_actions_type.py +++ b/launch/launch/some_actions_type.py @@ -12,21 +12,22 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Module for SomeActionsType type.""" +""" +Module for SomeActionsType type. -import collections.abc -from typing import Iterable -from typing import Union +.. deprecated:: 1.4.2 + Replaced by the more aptly named 'SomeEntitiesType' +""" -from .launch_description_entity import LaunchDescriptionEntity +import warnings +from .some_entities_type import SomeEntitiesType, SomeEntitiesType_types_tuple -SomeActionsType = Union[ - LaunchDescriptionEntity, - Iterable[LaunchDescriptionEntity], -] - -SomeActionsType_types_tuple = ( - LaunchDescriptionEntity, - collections.abc.Iterable, +warnings.warn( + "The 'SomeActionsType' type is deprecated. Use 'SomeEntitiesType' in your type" + ' annotations instead!', + UserWarning, ) + +SomeActionsType = SomeEntitiesType +SomeActionsType_types_tuple = SomeEntitiesType_types_tuple diff --git a/launch/launch/some_entities_type.py b/launch/launch/some_entities_type.py new file mode 100644 index 000000000..b42480184 --- /dev/null +++ b/launch/launch/some_entities_type.py @@ -0,0 +1,31 @@ +# Copyright 2018 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Module for SomeEntitiesType type.""" + +import collections.abc +from typing import Iterable +from typing import Union + +from .launch_description_entity import LaunchDescriptionEntity + +SomeEntitiesType = Union[ + LaunchDescriptionEntity, + Iterable[LaunchDescriptionEntity], +] + +SomeEntitiesType_types_tuple = ( + LaunchDescriptionEntity, + collections.abc.Iterable, +) diff --git a/launch/launch/substitutions/anon_name.py b/launch/launch/substitutions/anon_name.py index 09bf92d3f..2b9d2b9ee 100644 --- a/launch/launch/substitutions/anon_name.py +++ b/launch/launch/substitutions/anon_name.py @@ -14,8 +14,8 @@ """Module for the anonymous name substitution.""" -from typing import Iterable from typing import List +from typing import Sequence from typing import Text from ..frontend import expose_substitution @@ -41,7 +41,7 @@ def __init__(self, name: SomeSubstitutionsType) -> None: self.__name = normalize_to_list_of_substitutions(name) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `AnonName` substitution.""" if len(data) != 1: raise TypeError('anon substitution expects 1 argument') @@ -61,14 +61,10 @@ def perform(self, context: LaunchContext) -> Text: from ..utilities import perform_substitutions name = perform_substitutions(context, self.name) - if 'anon' not in context.launch_configurations: - context.launch_configurations['anon'] = {} - anon_context = context.launch_configurations['anon'] + if 'anon' + name not in context.launch_configurations: + context.launch_configurations['anon' + name] = self.compute_name(name) - if name not in anon_context: - anon_context[name] = self.compute_name(name) - - return anon_context[name] + return context.launch_configurations['anon' + name] def compute_name(self, id_value: Text) -> Text: """Get anonymous name based on id value.""" diff --git a/launch/launch/substitutions/boolean_substitution.py b/launch/launch/substitutions/boolean_substitution.py index 45593e76b..14b57ae60 100644 --- a/launch/launch/substitutions/boolean_substitution.py +++ b/launch/launch/substitutions/boolean_substitution.py @@ -15,6 +15,8 @@ """Module for boolean substitutions.""" from typing import Iterable +from typing import List +from typing import Sequence from typing import Text from .substitution_failure import SubstitutionFailure @@ -37,14 +39,14 @@ def __init__(self, value: SomeSubstitutionsType) -> None: self.__value = normalize_to_list_of_substitutions(value) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `NotSubstitution` substitution.""" if len(data) != 1: raise TypeError('not substitution expects 1 argument') return cls, {'value': data[0]} @property - def value(self) -> Substitution: + def value(self) -> List[Substitution]: """Getter for value.""" return self.__value @@ -74,19 +76,19 @@ def __init__(self, left: SomeSubstitutionsType, right: SomeSubstitutionsType) -> self.__right = normalize_to_list_of_substitutions(right) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `AndSubstitution` substitution.""" if len(data) != 2: raise TypeError('and substitution expects 2 arguments') return cls, {'left': data[0], 'right': data[1]} @property - def left(self) -> Substitution: + def left(self) -> List[Substitution]: """Getter for left.""" return self.__left @property - def right(self) -> Substitution: + def right(self) -> List[Substitution]: """Getter for right.""" return self.__right @@ -120,19 +122,19 @@ def __init__(self, left: SomeSubstitutionsType, right: SomeSubstitutionsType) -> self.__right = normalize_to_list_of_substitutions(right) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `OrSubstitution` substitution.""" if len(data) != 2: raise TypeError('and substitution expects 2 arguments') return cls, {'left': data[0], 'right': data[1]} @property - def left(self) -> Substitution: + def left(self) -> List[Substitution]: """Getter for left.""" return self.__left @property - def right(self) -> Substitution: + def right(self) -> List[Substitution]: """Getter for right.""" return self.__right @@ -178,13 +180,13 @@ def parse(cls, data: Iterable[SomeSubstitutionsType]): return cls, {'args': data} @property - def args(self) -> Substitution: + def args(self) -> List[List[Substitution]]: """Getter for args.""" return self.__args def describe(self) -> Text: """Return a description of this substitution as a string.""" - return f'AnySubstitution({" ".join(self.args)})' + return f'AnySubstitution({" ".join(str(arg) for arg in self.args)})' def perform(self, context: LaunchContext) -> Text: """Perform the substitution.""" @@ -224,13 +226,13 @@ def parse(cls, data: Iterable[SomeSubstitutionsType]): return cls, {'args': data} @property - def args(self) -> Substitution: + def args(self) -> List[List[Substitution]]: """Getter for args.""" return self.__args def describe(self) -> Text: """Return a description of this substitution as a string.""" - return f'AllSubstitution({" ".join(self.args)})' + return f'AllSubstitution({" ".join(str(arg) for arg in self.args)})' def perform(self, context: LaunchContext) -> Text: """Perform the substitution.""" diff --git a/launch/launch/substitutions/command.py b/launch/launch/substitutions/command.py index e918937ef..0aed7a9d6 100644 --- a/launch/launch/substitutions/command.py +++ b/launch/launch/substitutions/command.py @@ -17,9 +17,10 @@ import os import shlex import subprocess -from typing import Iterable from typing import List +from typing import Sequence from typing import Text +from typing import Union import launch.logging @@ -65,7 +66,7 @@ def __init__( self.__on_stderr = normalize_to_list_of_substitutions(on_stderr) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `Command` substitution.""" if len(data) < 1 or len(data) > 2: raise ValueError('command substitution expects 1 or 2 arguments') @@ -92,6 +93,7 @@ def perform(self, context: LaunchContext) -> Text: """Perform the substitution by running the command and capturing its output.""" from ..utilities import perform_substitutions # import here to avoid loop command_str = perform_substitutions(context, self.command) + command: Union[str, List[str]] if os.name != 'nt': command = shlex.split(command_str) else: diff --git a/launch/launch/substitutions/environment_variable.py b/launch/launch/substitutions/environment_variable.py index a6606b96d..26a5fb70d 100644 --- a/launch/launch/substitutions/environment_variable.py +++ b/launch/launch/substitutions/environment_variable.py @@ -14,9 +14,9 @@ """Module for the EnvironmentVariable substitution.""" -from typing import Iterable from typing import List from typing import Optional +from typing import Sequence from typing import Text from .substitution_failure import SubstitutionFailure @@ -62,7 +62,7 @@ def __init__( self.__default_value = default_value @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `EnviromentVariable` substitution.""" if len(data) < 1 or len(data) > 2: raise TypeError('env substitution expects 1 or 2 arguments') @@ -77,7 +77,7 @@ def name(self) -> List[Substitution]: return self.__name @property - def default_value(self) -> List[Substitution]: + def default_value(self) -> Optional[List[Substitution]]: """Getter for default_value.""" return self.__default_value @@ -88,8 +88,8 @@ def describe(self) -> Text: def perform(self, context: LaunchContext) -> Text: """Perform the substitution by looking up the environment variable.""" from ..utilities import perform_substitutions # import here to avoid loop - default_value = self.default_value - if default_value is not None: + default_value: Optional[str] = None + if self.default_value is not None: default_value = perform_substitutions(context, self.default_value) name = perform_substitutions(context, self.name) value = context.environment.get( diff --git a/launch/launch/substitutions/equals_substitution.py b/launch/launch/substitutions/equals_substitution.py index 0fb0b9f2f..02c91d071 100644 --- a/launch/launch/substitutions/equals_substitution.py +++ b/launch/launch/substitutions/equals_substitution.py @@ -17,8 +17,11 @@ import math from typing import Any +from typing import cast from typing import Iterable +from typing import List from typing import Optional +from typing import Sequence from typing import Text from typing import Union @@ -79,23 +82,31 @@ def __init__( else: right = str(right) - self.__left = normalize_to_list_of_substitutions(left) - self.__right = normalize_to_list_of_substitutions(right) + # mypy is unable to understand that if we passed in the `else` branch + # above, left & right must be substitutions. Unfortunately due to the + # way is_substitution is written, it's hard to get mypy to typecheck + # it correctly, so cast here. + self.__left = normalize_to_list_of_substitutions( + cast(Union[str, Substitution, Sequence[Union[str, Substitution]]], left) + ) + self.__right = normalize_to_list_of_substitutions( + cast(Union[str, Substitution, Sequence[Union[str, Substitution]]], right) + ) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `EqualsSubstitution` substitution.""" if len(data) != 2: raise TypeError('and substitution expects 2 arguments') return cls, {'left': data[0], 'right': data[1]} @property - def left(self) -> Substitution: + def left(self) -> List[Substitution]: """Getter for left.""" return self.__left @property - def right(self) -> Substitution: + def right(self) -> List[Substitution]: """Getter for right.""" return self.__right diff --git a/launch/launch/substitutions/find_executable.py b/launch/launch/substitutions/find_executable.py index 56bac7466..1124428ac 100644 --- a/launch/launch/substitutions/find_executable.py +++ b/launch/launch/substitutions/find_executable.py @@ -14,11 +14,11 @@ """Module for the FindExecutable substitution.""" -from typing import Iterable from typing import List +from typing import Sequence from typing import Text -from osrf_pycommon.process_utils import which +from osrf_pycommon.process_utils import which # type: ignore from .substitution_failure import SubstitutionFailure from ..frontend import expose_substitution @@ -43,7 +43,7 @@ def __init__(self, *, name: SomeSubstitutionsType) -> None: self.__name = normalize_to_list_of_substitutions(name) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `FindExecutable` substitution.""" if len(data) != 1: raise AttributeError('find-exec substitution expects 1 argument') diff --git a/launch/launch/substitutions/launch_configuration.py b/launch/launch/substitutions/launch_configuration.py index 41fdf80fa..4698129f9 100644 --- a/launch/launch/substitutions/launch_configuration.py +++ b/launch/launch/substitutions/launch_configuration.py @@ -19,6 +19,7 @@ from typing import Iterable from typing import List from typing import Optional +from typing import Sequence from typing import Text from typing import Union @@ -44,6 +45,7 @@ def __init__( from ..utilities import normalize_to_list_of_substitutions self.__variable_name = normalize_to_list_of_substitutions(variable_name) + self.__default: Optional[List[Substitution]] if default is None: self.__default = default else: @@ -60,12 +62,10 @@ def __init__( else: str_normalized_default.append(str(item)) # use normalize_to_list_of_substitutions to convert str to TextSubstitution's too - self.__default = \ - normalize_to_list_of_substitutions( - str_normalized_default) # type: List[Substitution] + self.__default = normalize_to_list_of_substitutions(str_normalized_default) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `FindExecutable` substitution.""" if len(data) < 1 or len(data) > 2: raise TypeError('var substitution expects 1 or 2 arguments') diff --git a/launch/launch/substitutions/launch_log_dir.py b/launch/launch/substitutions/launch_log_dir.py new file mode 100644 index 000000000..dbe29847d --- /dev/null +++ b/launch/launch/substitutions/launch_log_dir.py @@ -0,0 +1,49 @@ +# Copyright 2022 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Module for the LaunchLogDir substitution.""" + +from typing import Sequence +from typing import Text + +from ..frontend.expose import expose_substitution +from ..launch_context import LaunchContext +from ..logging import launch_config as launch_logging_config +from ..some_substitutions_type import SomeSubstitutionsType +from ..substitution import Substitution + + +@expose_substitution('launch_log_dir') +@expose_substitution('log_dir') +class LaunchLogDir(Substitution): + """Substitution that returns the absolute path to the current launch log directory.""" + + def __init__(self) -> None: + """Create a LaunchLogDir substitution.""" + super().__init__() + + @classmethod + def parse(cls, data: Sequence[SomeSubstitutionsType]): + """Parse `LaunchLogDir` substitution.""" + if len(data) != 0: + raise TypeError("launch_log_dir/log_dir substitution doesn't expect arguments") + return cls, {} + + def describe(self) -> Text: + """Return a description of this substitution as a string.""" + return 'LaunchLogDir()' + + def perform(self, context: LaunchContext) -> Text: + """Perform the substitution by returning the path to the current launch log directory.""" + return launch_logging_config.log_dir diff --git a/launch/launch/substitutions/path_join_substitution.py b/launch/launch/substitutions/path_join_substitution.py index 05117ae39..c75e0c932 100644 --- a/launch/launch/substitutions/path_join_substitution.py +++ b/launch/launch/substitutions/path_join_substitution.py @@ -18,9 +18,9 @@ from typing import Iterable from typing import List from typing import Text +from typing import Union from ..launch_context import LaunchContext -from ..some_substitutions_type import SomeSubstitutionsType from ..substitution import Substitution from ..utilities import normalize_to_list_of_substitutions from ..utilities import perform_substitutions @@ -59,7 +59,7 @@ class PathJoinSubstitution(Substitution): '/home/user/dir/cfg/config_my_map.yml' """ - def __init__(self, substitutions: Iterable[SomeSubstitutionsType]) -> None: + def __init__(self, substitutions: Iterable[Union[Text, Substitution]]) -> None: """Create a PathJoinSubstitution.""" from ..utilities import normalize_to_list_of_substitutions self.__substitutions = [ diff --git a/launch/launch/substitutions/python_expression.py b/launch/launch/substitutions/python_expression.py index 3ee4fe957..7daad3571 100644 --- a/launch/launch/substitutions/python_expression.py +++ b/launch/launch/substitutions/python_expression.py @@ -15,9 +15,14 @@ """Module for the PythonExpression substitution.""" import collections.abc +<<<<<<< HEAD import math from typing import Iterable +======= +import importlib +>>>>>>> 06dc66b (Improve type checking (#679)) from typing import List +from typing import Sequence from typing import Text from ..frontend import expose_substitution @@ -51,11 +56,37 @@ def __init__(self, expression: SomeSubstitutionsType) -> None: self.__expression = normalize_to_list_of_substitutions(expression) @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `PythonExpression` substitution.""" +<<<<<<< HEAD if len(data) != 1: raise TypeError('eval substitution expects 1 argument') return cls, {'expression': data[0]} +======= + if len(data) < 1 or len(data) > 2: + raise TypeError('eval substitution expects 1 or 2 arguments') + kwargs = {} + kwargs['expression'] = data[0] + if len(data) == 2: + # We get a text substitution from XML, + # whose contents are comma-separated module names + kwargs['python_modules'] = [] + # Check if we got empty list from XML + # Ensure that we got a list! + assert(not isinstance(data[1], str)) + assert(not isinstance(data[1], Substitution)) + # Modules + modules = list(data[1]) + if len(modules) > 0: + # XXX: What is going on here: the type annotation says we should get + # a either strings or substitutions, but this says that we're + # getting a substitution always? + # Moreover, `perform` is called with `None`, which is not acceptable + # for any substitution as far as I know (should be an empty launch context?) + modules_str = modules[0].perform(None) # type: ignore + kwargs['python_modules'] = [module.strip() for module in modules_str.split(',')] + return cls, kwargs +>>>>>>> 06dc66b (Improve type checking (#679)) @property def expression(self) -> List[Substitution]: diff --git a/launch/launch/substitutions/this_launch_file.py b/launch/launch/substitutions/this_launch_file.py index 96a12e1a3..178953999 100644 --- a/launch/launch/substitutions/this_launch_file.py +++ b/launch/launch/substitutions/this_launch_file.py @@ -14,7 +14,7 @@ """Module for the ThisLaunchFile substitution.""" -from typing import Iterable +from typing import Sequence from typing import Text from .substitution_failure import SubstitutionFailure @@ -33,8 +33,13 @@ def __init__(self) -> None: super().__init__() @classmethod +<<<<<<< HEAD def parse(cls, data: Iterable[SomeSubstitutionsType]): """Parse `EnviromentVariable` substitution.""" +======= + def parse(cls, data: Sequence[SomeSubstitutionsType]): + """Parse `ThisLaunchFile` substitution.""" +>>>>>>> 06dc66b (Improve type checking (#679)) if len(data) != 0: raise TypeError("filename substitution doesn't expect arguments") return cls, {} diff --git a/launch/launch/substitutions/this_launch_file_dir.py b/launch/launch/substitutions/this_launch_file_dir.py index 10917aaf9..08004cf64 100644 --- a/launch/launch/substitutions/this_launch_file_dir.py +++ b/launch/launch/substitutions/this_launch_file_dir.py @@ -14,7 +14,7 @@ """Module for the ThisLaunchFileDir substitution.""" -from typing import Iterable +from typing import Sequence from typing import Text from .substitution_failure import SubstitutionFailure @@ -33,8 +33,13 @@ def __init__(self) -> None: super().__init__() @classmethod +<<<<<<< HEAD def parse(cls, data: Iterable[SomeSubstitutionsType]): """Parse `EnviromentVariable` substitution.""" +======= + def parse(cls, data: Sequence[SomeSubstitutionsType]): + """Parse `ThisLaunchFileDir` substitution.""" +>>>>>>> 06dc66b (Improve type checking (#679)) if len(data) != 0: raise TypeError("dirname substitution doesn't expect arguments") return cls, {} diff --git a/launch/launch/utilities/__init__.py b/launch/launch/utilities/__init__.py index f3efb4de5..ca618adee 100644 --- a/launch/launch/utilities/__init__.py +++ b/launch/launch/utilities/__init__.py @@ -17,6 +17,7 @@ from .class_tools_impl import is_a, is_a_subclass, isclassinstance from .create_future_impl import create_future from .ensure_argument_type_impl import ensure_argument_type +from .normalize_to_list_of_entities_impl import normalize_to_list_of_entities from .normalize_to_list_of_substitutions_impl import normalize_to_list_of_substitutions from .perform_substitutions_impl import perform_substitutions from .signal_management import AsyncSafeSignalManager @@ -31,5 +32,6 @@ 'perform_substitutions', 'AsyncSafeSignalManager', 'normalize_to_list_of_substitutions', + 'normalize_to_list_of_entities', 'visit_all_entities_and_collect_futures', ] diff --git a/launch/launch/utilities/normalize_to_list_of_entities_impl.py b/launch/launch/utilities/normalize_to_list_of_entities_impl.py new file mode 100644 index 000000000..a41652d41 --- /dev/null +++ b/launch/launch/utilities/normalize_to_list_of_entities_impl.py @@ -0,0 +1,33 @@ +# Copyright 2023 Toyota Motor Corporation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Module for the normalize_to_list_of_entities() utility function.""" + +from typing import Iterable +from typing import List + +from ..launch_description_entity import LaunchDescriptionEntity +from ..some_entities_type import SomeEntitiesType + + +def normalize_to_list_of_entities(entities: Iterable[SomeEntitiesType]) ->\ + List[LaunchDescriptionEntity]: + """Return a list of Substitutions given a variety of starting inputs.""" + flattened: List[LaunchDescriptionEntity] = [] + for entity in entities: + if isinstance(entity, LaunchDescriptionEntity): + flattened.append(entity) + else: + flattened.extend(entity) + return flattened diff --git a/launch/launch/utilities/signal_management.py b/launch/launch/utilities/signal_management.py index 621ddcd89..ebee12033 100644 --- a/launch/launch/utilities/signal_management.py +++ b/launch/launch/utilities/signal_management.py @@ -73,11 +73,11 @@ def __init__( :param loop: event loop that will handle the signals. """ - self.__parent = None # type: AsyncSafeSignalManager - self.__loop = loop # type: asyncio.AbstractEventLoop - self.__background_loop = None # type: Optional[asyncio.AbstractEventLoop] - self.__handlers = {} # type: dict - self.__prev_wakeup_handle = -1 # type: Union[int, socket.socket] + self.__parent: Optional[AsyncSafeSignalManager] = None + self.__loop: asyncio.AbstractEventLoop = loop + self.__background_loop: Optional[asyncio.AbstractEventLoop] = None + self.__handlers: dict = {} + self.__prev_wakeup_handle: Union[int, socket.socket] = -1 self.__wsock = None self.__rsock = None self.__close_sockets = None diff --git a/launch/launch/utilities/type_utils.py b/launch/launch/utilities/type_utils.py index 9fd795e3a..3f4aa2a21 100644 --- a/launch/launch/utilities/type_utils.py +++ b/launch/launch/utilities/type_utils.py @@ -26,7 +26,9 @@ from typing import Type from typing import Union -import yaml +# yaml has type annotations in typeshed, but those cannot be installed via rosdep +# since there is no definition for types-PyYAML +import yaml # type: ignore from .ensure_argument_type_impl import ensure_argument_type from .normalize_to_list_of_substitutions_impl import normalize_to_list_of_substitutions @@ -200,6 +202,7 @@ def is_instance_of( """ if data_type is None: return is_instance_of_valid_type(value, can_be_str=can_be_str) + type_obj: Union[Tuple[Type[str], AllowedTypesType], AllowedTypesType] type_obj, is_list = extract_type(data_type) if can_be_str: type_obj = (str, type_obj) @@ -342,12 +345,15 @@ def get_typed_value( f"Cannot convert input '{value}' of type '{type(value)}' to" f" '{data_type}'" ) - output: AllowedValueType = coerce_list(value, data_type, can_be_str=can_be_str) + return coerce_list(value, data_type, can_be_str=can_be_str) else: - output = coerce_to_type(value, data_type, can_be_str=can_be_str) - return output + return coerce_to_type(value, data_type, can_be_str=can_be_str) +# Unfortunately, mypy is unable to correctly infer that `is_substitution` can +# only return True when the passed tpe is either a substitution or a mixed +# list of strings and substitutions. Indeed, there is no way that I could find +# using overloads to describe "anything else than the above two types". def is_substitution(x): """ Return `True` if `x` is some substitution. @@ -517,7 +523,7 @@ def perform_typed_substitution( context: LaunchContext, value: NormalizedValueType, data_type: Optional[AllowedTypesType] -) -> AllowedValueType: +) -> StrSomeValueType: """ Perform a normalized typed substitution. diff --git a/launch/package.xml b/launch/package.xml index c333351d6..3cbde230e 100644 --- a/launch/package.xml +++ b/launch/package.xml @@ -23,6 +23,7 @@ ament_copyright ament_flake8 ament_pep257 + ament_mypy python3-pytest diff --git a/launch/setup.cfg b/launch/setup.cfg new file mode 100644 index 000000000..976ba0294 --- /dev/null +++ b/launch/setup.cfg @@ -0,0 +1,2 @@ +[mypy] +ignore_missing_imports = True diff --git a/launch/test/launch/frontend/test_substitutions.py b/launch/test/launch/frontend/test_substitutions.py index 9fabbc6a9..ceac250ab 100644 --- a/launch/test/launch/frontend/test_substitutions.py +++ b/launch/test/launch/frontend/test_substitutions.py @@ -28,6 +28,7 @@ from launch.substitutions import PythonExpression from launch.substitutions import TextSubstitution from launch.substitutions import ThisLaunchFileDir +from launch.utilities import normalize_to_list_of_substitutions import pytest @@ -57,7 +58,8 @@ def test_text_only(): def perform_substitutions_without_context(subs: List[Substitution]): - return ''.join([sub.perform(None) for sub in subs]) + # XXX : Why is it possible to pass `None` to perform? + return ''.join([sub.perform(None) for sub in subs]) # type: ignore class CustomSubstitution(Substitution): @@ -214,7 +216,10 @@ def test_eval_subst_of_math_expr(): def expand_cmd_subs(cmd_subs: List[SomeSubstitutionsType]): - return [perform_substitutions_without_context(x) for x in cmd_subs] + return [ + perform_substitutions_without_context(normalize_to_list_of_substitutions(x)) + for x in cmd_subs + ] def test_parse_if_substitutions(): diff --git a/launch/test/launch/test_event_handler.py b/launch/test/launch/test_event_handler.py index 3cce1c115..040abd58c 100644 --- a/launch/test/launch/test_event_handler.py +++ b/launch/test/launch/test_event_handler.py @@ -17,7 +17,7 @@ from launch import EventHandler from launch import LaunchContext from launch import LaunchDescriptionEntity -from launch import SomeActionsType_types_tuple +from launch import SomeEntitiesType_types_tuple import pytest @@ -40,7 +40,7 @@ class MockEvent: mock_event = MockEvent() context = LaunchContext() entities = eh.handle(mock_event, context) - assert isinstance(entities, SomeActionsType_types_tuple) + assert isinstance(entities, SomeEntitiesType_types_tuple) assert len(entities) == 1 assert context.locals.event == mock_event diff --git a/launch/test/launch/utilities/test_signal_management.py b/launch/test/launch/utilities/test_signal_management.py index 62dffbc3a..d95d2a9c7 100644 --- a/launch/test/launch/utilities/test_signal_management.py +++ b/launch/test/launch/utilities/test_signal_management.py @@ -16,8 +16,8 @@ import asyncio import functools -import platform import signal +import sys from launch.utilities import AsyncSafeSignalManager @@ -43,7 +43,7 @@ def _wrapper(*args, **kwargs): return _decorator -if platform.system() == 'Windows': +if sys.platform == 'win32': # NOTE(hidmic): this is risky, but we have few options. SIGNAL = signal.SIGINT ANOTHER_SIGNAL = signal.SIGBREAK @@ -51,7 +51,7 @@ def _wrapper(*args, **kwargs): SIGNAL = signal.SIGUSR1 ANOTHER_SIGNAL = signal.SIGUSR2 -if not hasattr(signal, 'raise_signal'): +if sys.version_info < (3, 8): # Only available for Python 3.8+ def raise_signal(signum): import os diff --git a/launch/test/launch/utilities/test_type_utils.py b/launch/test/launch/utilities/test_type_utils.py index 1e7f9cafe..f85f0cf33 100644 --- a/launch/test/launch/utilities/test_type_utils.py +++ b/launch/test/launch/utilities/test_type_utils.py @@ -340,8 +340,14 @@ def test_coercing_list_using_yaml_rules(coerce_list_impl): 'coerce_list_impl', ( coerce_list, + # There is a bit of confusion here, since we pass in a type value + # but then attempt to use it as a type variable in the annotation + # List[data_type]. In general mypy does not support very well this + # sort of dynamic typing, so ignore for now. The better way to type + # this is probably to use TypeVars and / or overloads but I couldn't + # quite figure it out. lambda value, data_type=None, can_be_str=False: get_typed_value( - value, List[data_type], can_be_str=can_be_str), + value, List[data_type], can_be_str=can_be_str), # type: ignore ), ids=[ 'testing coerce_list implementation', diff --git a/launch/test/test_mypy.py b/launch/test/test_mypy.py new file mode 100644 index 000000000..31db1869b --- /dev/null +++ b/launch/test/test_mypy.py @@ -0,0 +1,23 @@ +# Copyright 2022 Toyota Motor Corporation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from ament_mypy.main import main +import pytest + + +@pytest.mark.mypy +@pytest.mark.linter +def test_mypy(): + rc = main(argv=[]) + assert rc == 0, 'Found type errors!' diff --git a/launch_testing/launch_testing/actions/test.py b/launch_testing/launch_testing/actions/test.py index f77d8c25d..bebc537b6 100644 --- a/launch_testing/launch_testing/actions/test.py +++ b/launch_testing/launch_testing/actions/test.py @@ -19,7 +19,7 @@ from typing import Union from launch import LaunchContext -from launch import SomeActionsType +from launch import SomeEntitiesType from launch import SomeSubstitutionsType from launch.action import Action from launch.actions import ExecuteProcess @@ -56,7 +56,9 @@ def timeout(self): """Getter for timeout.""" return self.__timeout - def __on_process_exit(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: + def __on_process_exit( + self, event: Event, context: LaunchContext + ) -> Optional[SomeEntitiesType]: """On shutdown event.""" if self.__timer: self.__timer.cancel() diff --git a/launch_testing/launch_testing/event_handlers/stdout_ready_listener.py b/launch_testing/launch_testing/event_handlers/stdout_ready_listener.py index d89e530ce..af02ba5cc 100644 --- a/launch_testing/launch_testing/event_handlers/stdout_ready_listener.py +++ b/launch_testing/launch_testing/event_handlers/stdout_ready_listener.py @@ -19,7 +19,7 @@ from launch.actions import ExecuteProcess from launch.event_handlers import OnProcessIO -from launch.some_actions_type import SomeActionsType +from launch.some_entities_type import SomeEntitiesType class StdoutReadyListener(OnProcessIO): @@ -36,7 +36,7 @@ def __init__( *, target_action: Optional[ExecuteProcess] = None, ready_txt: Text, - actions: [SomeActionsType] + actions: [SomeEntitiesType] ): self.__ready_txt = ready_txt self.__actions = actions @@ -50,7 +50,7 @@ def __on_stdout(self, process_io): if self.__ready_txt in process_io.text.decode(): return self.__actions - def describe(self) -> Tuple[Text, List[SomeActionsType]]: + def describe(self) -> Tuple[Text, List[SomeEntitiesType]]: """Return the description list with 0 as a string, and then LaunchDescriptionEntity's.""" description = super().describe()[0] return (