From 53828e7829e9ca3ebb1091d54a10801a6cb24a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Wed, 30 Nov 2022 10:53:30 +0900 Subject: [PATCH 01/53] Add test for type checking --- launch/package.xml | 1 + launch/test/test_mypy.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 launch/test/test_mypy.py diff --git a/launch/package.xml b/launch/package.xml index b46221a99..5e2278ed5 100644 --- a/launch/package.xml +++ b/launch/package.xml @@ -25,6 +25,7 @@ ament_copyright ament_flake8 ament_pep257 + ament_mypy python3-pytest diff --git a/launch/test/test_mypy.py b/launch/test/test_mypy.py new file mode 100644 index 000000000..056094604 --- /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. + +import pytest +from ament_mypy.main import main + + +@pytest.mark.mypy +@pytest.mark.linter +def test_mypy(): + rc = main(argv=[]) + assert rc == 0, "Found type errors!" From 51f3aab48979b2d44f61ac39e7faf4b83d7f7c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 1 Dec 2022 10:18:35 +0900 Subject: [PATCH 02/53] Fix some type errors --- launch/launch/utilities/signal_management.py | 10 +++++----- launch/launch/utilities/type_utils.py | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) 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..69b87eab1 100644 --- a/launch/launch/utilities/type_utils.py +++ b/launch/launch/utilities/type_utils.py @@ -200,6 +200,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,10 +343,9 @@ 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) def is_substitution(x): @@ -517,7 +517,7 @@ def perform_typed_substitution( context: LaunchContext, value: NormalizedValueType, data_type: Optional[AllowedTypesType] -) -> AllowedValueType: +) -> StrSomeValueType: """ Perform a normalized typed substitution. From 736293216e71a22224241780fffe100556eae725 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 5 Jan 2023 17:05:09 +0900 Subject: [PATCH 03/53] Fix a few type errors: - Declare the type of a dictionary - Add missing return - Annoyingly the return type of get_entity is a union of all possible types. Ideally, get_entity would return the same type as what's passed in by allowed_data_types (and possibly string) but that would require some advanced type wrangling to achieve template-like functionality. For now, use casts where necessary. --- launch/launch/action.py | 8 +++++--- launch/launch/actions/append_environment_variable.py | 9 +++++---- launch/launch/event_handler.py | 1 + 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/launch/launch/action.py b/launch/launch/action.py index fd7146576..84ba32a96 100644 --- a/launch/launch/action.py +++ b/launch/launch/action.py @@ -19,6 +19,8 @@ from typing import Optional from typing import Text from typing import Tuple +from typing import Dict +from typing import cast from .condition import Condition from .launch_context import LaunchContext @@ -60,9 +62,9 @@ def parse(entity: 'Entity', parser: 'Parser'): # Import here for avoiding cyclic imports. from .conditions import IfCondition from .conditions import UnlessCondition - if_cond = entity.get_attr('if', optional=True) - unless_cond = entity.get_attr('unless', optional=True) - kwargs = {} + if_cond = cast(str, entity.get_attr('if', optional=True)) + unless_cond = cast(str, entity.get_attr('unless', optional=True)) + 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/append_environment_variable.py b/launch/launch/actions/append_environment_variable.py index 0c0c75367..1cc21afb0 100644 --- a/launch/launch/actions/append_environment_variable.py +++ b/launch/launch/actions/append_environment_variable.py @@ -17,6 +17,7 @@ import os from typing import List from typing import Union +from typing import cast from ..action import Action from ..frontend import Entity @@ -75,14 +76,14 @@ def parse( ): """Parse an 'append_env' entity.""" _, kwargs = super().parse(entity, parser) - kwargs['name'] = parser.parse_substitution(entity.get_attr('name')) - kwargs['value'] = parser.parse_substitution(entity.get_attr('value')) + kwargs['name'] = parser.parse_substitution(cast(str, entity.get_attr('name'))) + kwargs['value'] = parser.parse_substitution(cast(str, entity.get_attr('value'))) prepend = entity.get_attr('prepend', optional=True, data_type=bool, can_be_str=True) if prepend is not None: - kwargs['prepend'] = parser.parse_if_substitutions(prepend) + kwargs['prepend'] = parser.parse_if_substitutions(cast(Union[str, bool], prepend)) separator = entity.get_attr('separator', optional=True) if separator is not None: - kwargs['separator'] = parser.parse_substitution(separator) + kwargs['separator'] = parser.parse_substitution(cast(str, separator)) return cls, kwargs @property diff --git a/launch/launch/event_handler.py b/launch/launch/event_handler.py index 16902671a..bc98f6ae6 100644 --- a/launch/launch/event_handler.py +++ b/launch/launch/event_handler.py @@ -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): From 430f594a1d7f7dd32a6366064c7bff9cb82adbb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 5 Jan 2023 17:49:11 +0900 Subject: [PATCH 04/53] Fix declare launch argument --- .../launch/actions/declare_launch_argument.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/launch/launch/actions/declare_launch_argument.py b/launch/launch/actions/declare_launch_argument.py index da1c63cd7..54aa2fd13 100644 --- a/launch/launch/actions/declare_launch_argument.py +++ b/launch/launch/actions/declare_launch_argument.py @@ -14,10 +14,10 @@ """Module for the DeclareLaunchArgument action.""" -from typing import Iterable from typing import List from typing import Optional from typing import Text +from typing import cast import launch.logging @@ -109,7 +109,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.""" @@ -166,17 +166,19 @@ def parse( ): """Parse `arg` tag.""" _, kwargs = super().parse(entity, parser) - kwargs['name'] = parser.escape_characters(entity.get_attr('name')) + kwargs['name'] = parser.escape_characters(cast(str, entity.get_attr('name'))) default_value = entity.get_attr('default', optional=True) if default_value is not None: - kwargs['default_value'] = parser.parse_substitution(default_value) + kwargs['default_value'] = parser.parse_substitution(cast(str, default_value)) description = entity.get_attr('description', optional=True) if description is not None: - kwargs['description'] = parser.escape_characters(description) - choices = entity.get_attr('choice', data_type=List[Entity], optional=True) + kwargs['description'] = parser.escape_characters(cast(str, description)) + # TODO: What to do here? get_attr is supposed to parse scalar / list of scalars, but in + # this case asks for a list of entities? + choices = cast(List[Entity], entity.get_attr('choice', data_type=List[Entity], optional=True)) # type: ignore if choices is not None: kwargs['choices'] = [ - parser.escape_characters(choice.get_attr('value')) for choice in choices + parser.escape_characters(cast(str, choice.get_attr('value'))) for choice in choices ] return cls, kwargs @@ -196,7 +198,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 From 9d3f77a938ffb2b2aad253715632139b36529038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 5 Jan 2023 17:49:54 +0900 Subject: [PATCH 05/53] Fixup execute_local - Touchup process_targeted_events and shutdown_process to accept a ExecuteLocal instead of ExecuteProcess - Still an issue surrounding OnExit - Many minor fixes --- launch/launch/actions/execute_local.py | 31 ++++++++++++------- .../events/process/process_targeted_event.py | 8 ++--- .../launch/events/process/shutdown_process.py | 6 ++-- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/launch/launch/actions/execute_local.py b/launch/launch/actions/execute_local.py index 553d31120..2258e3140 100644 --- a/launch/launch/actions/execute_local.py +++ b/launch/launch/actions/execute_local.py @@ -189,9 +189,14 @@ 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): + # 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(self.__output) + else: + self.__output = tmp_output self.__output_format = output_format self.__log_cmd = log_cmd @@ -351,12 +356,12 @@ 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 # buffering. - buffer.info( + logger.info( self.__output_format.format(line=to_write, this=self) ) else: @@ -402,7 +407,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 @@ -586,7 +591,7 @@ async def __execute_process(self, context: LaunchContext) -> None: )) 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`) @@ -614,7 +619,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]]: """ @@ -669,7 +674,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, @@ -684,9 +691,11 @@ 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/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) From 2beeb747460a5cdf7aa49f72fd4bb0fb646fe4a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 6 Jan 2023 10:27:36 +0900 Subject: [PATCH 06/53] Fixup path substitution Just change the input type --- launch/launch/substitutions/path_join_substitution.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launch/launch/substitutions/path_join_substitution.py b/launch/launch/substitutions/path_join_substitution.py index ab3d6f273..0d9aa4d25 100644 --- a/launch/launch/substitutions/path_join_substitution.py +++ b/launch/launch/substitutions/path_join_substitution.py @@ -17,16 +17,16 @@ import os from typing import Iterable from typing import Text +from typing import Union from ..launch_context import LaunchContext -from ..some_substitutions_type import SomeSubstitutionsType from ..substitution import Substitution class PathJoinSubstitution(Substitution): """Substitution that join paths, in a platform independent way.""" - 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 = normalize_to_list_of_substitutions(substitutions) From 550f034cae030c4a4cf3575dc8c300a14241c3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 6 Jan 2023 10:38:31 +0900 Subject: [PATCH 07/53] Fixup execute process Mostly casts around get attribute --- launch/launch/actions/execute_process.py | 34 +++++++++++++----------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index ea85c5e81..913b8cafb 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -20,6 +20,7 @@ from typing import List from typing import Optional from typing import Text +from typing import cast from .execute_local import ExecuteLocal @@ -28,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 @@ -250,7 +253,7 @@ def _parse_cmdline( :returns: a list of command line arguments. """ result_args = [] - arg = [] + arg: List[Substitution] = [] def _append_arg(): nonlocal arg @@ -315,35 +318,35 @@ def parse( ignore = [] if 'cmd' not in ignore: - kwargs['cmd'] = cls._parse_cmdline(entity.get_attr('cmd'), parser) + kwargs['cmd'] = cls._parse_cmdline(cast(str, entity.get_attr('cmd')), parser) if 'cwd' not in ignore: - cwd = entity.get_attr('cwd', optional=True) + cwd = cast(Optional[str], entity.get_attr('cwd', optional=True)) if cwd is not None: kwargs['cwd'] = parser.parse_substitution(cwd) if 'name' not in ignore: - name = entity.get_attr('name', optional=True) + name = cast(Optional[str], entity.get_attr('name', optional=True)) if name is not None: kwargs['name'] = parser.parse_substitution(name) if 'prefix' not in ignore: - prefix = entity.get_attr('launch-prefix', optional=True) + prefix = cast(Optional[str], entity.get_attr('launch-prefix', optional=True)) if prefix is not None: kwargs['prefix'] = parser.parse_substitution(prefix) if 'output' not in ignore: - output = entity.get_attr('output', optional=True) + output = cast(Optional[str], entity.get_attr('output', optional=True)) if output is not None: kwargs['output'] = parser.parse_substitution(output) if 'respawn' not in ignore: - respawn = entity.get_attr('respawn', optional=True) + respawn = cast(Optional[str], entity.get_attr('respawn', optional=True)) if respawn is not None: kwargs['respawn'] = parser.parse_substitution(respawn) if 'respawn_delay' not in ignore: - respawn_delay = entity.get_attr('respawn_delay', data_type=float, optional=True) + respawn_delay = cast(Optional[float], entity.get_attr('respawn_delay', data_type=float, optional=True)) if respawn_delay is not None: if respawn_delay < 0.0: raise ValueError( @@ -353,7 +356,7 @@ def parse( kwargs['respawn_delay'] = respawn_delay if 'sigkill_timeout' not in ignore: - sigkill_timeout = entity.get_attr('sigkill_timeout', data_type=float, optional=True) + sigkill_timeout = cast(Optional[float], entity.get_attr('sigkill_timeout', data_type=float, optional=True)) if sigkill_timeout is not None: if sigkill_timeout < 0.0: raise ValueError( @@ -363,7 +366,7 @@ def parse( kwargs['sigkill_timeout'] = str(sigkill_timeout) if 'sigterm_timeout' not in ignore: - sigterm_timeout = entity.get_attr('sigterm_timeout', data_type=float, optional=True) + sigterm_timeout = cast(Optional[float], entity.get_attr('sigterm_timeout', data_type=float, optional=True)) if sigterm_timeout is not None: if sigterm_timeout < 0.0: raise ValueError( @@ -373,12 +376,12 @@ def parse( kwargs['sigterm_timeout'] = str(sigterm_timeout) if 'shell' not in ignore: - shell = entity.get_attr('shell', data_type=bool, optional=True) + shell = cast(Optional[bool], entity.get_attr('shell', data_type=bool, optional=True)) if shell is not None: kwargs['shell'] = shell if 'emulate_tty' not in ignore: - emulate_tty = entity.get_attr('emulate_tty', data_type=bool, optional=True) + emulate_tty = cast(Optional[bool], entity.get_attr('emulate_tty', data_type=bool, optional=True)) if emulate_tty is not None: kwargs['emulate_tty'] = emulate_tty @@ -386,11 +389,12 @@ def parse( # Conditions won't be allowed in the `env` tag. # If that feature is needed, `set_enviroment_variable` and # `unset_enviroment_variable` actions should be used. - env = entity.get_attr('env', data_type=List[Entity], optional=True) + # TODO: Fixup the data_type annotation + env = cast(Optional[List[Entity]], entity.get_attr('env', data_type=List[Entity], optional=True)) # type: ignore if env is not None: kwargs['additional_env'] = { - tuple(parser.parse_substitution(e.get_attr('name'))): - parser.parse_substitution(e.get_attr('value')) for e in env + tuple(parser.parse_substitution(cast(str, e.get_attr('name')))): + parser.parse_substitution(cast(str, e.get_attr('value'))) for e in env } for e in env: e.assert_entity_completely_parsed() From 9f070d4fc3325fbafd837e638b5e3e6a54c6fa29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 6 Jan 2023 12:01:12 +0900 Subject: [PATCH 08/53] Use overloads to make get_attr type safe The overloads allow us to properly distinguish between: - Data type passed or not passed (returns str) - Optional set to true or false (returns Optional[T] or T) Also remove the now redundant casts. --- launch/launch/action.py | 5 +- .../actions/append_environment_variable.py | 9 ++-- .../launch/actions/declare_launch_argument.py | 13 ++--- launch/launch/actions/execute_process.py | 29 +++++------ launch/launch/frontend/entity.py | 51 ++++++++++++++++--- 5 files changed, 68 insertions(+), 39 deletions(-) diff --git a/launch/launch/action.py b/launch/launch/action.py index 84ba32a96..ead786ae9 100644 --- a/launch/launch/action.py +++ b/launch/launch/action.py @@ -20,7 +20,6 @@ from typing import Text from typing import Tuple from typing import Dict -from typing import cast from .condition import Condition from .launch_context import LaunchContext @@ -62,8 +61,8 @@ def parse(entity: 'Entity', parser: 'Parser'): # Import here for avoiding cyclic imports. from .conditions import IfCondition from .conditions import UnlessCondition - if_cond = cast(str, entity.get_attr('if', optional=True)) - unless_cond = cast(str, entity.get_attr('unless', optional=True)) + if_cond = entity.get_attr('if', optional=True) + unless_cond = entity.get_attr('unless', optional=True) 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") diff --git a/launch/launch/actions/append_environment_variable.py b/launch/launch/actions/append_environment_variable.py index 1cc21afb0..0c0c75367 100644 --- a/launch/launch/actions/append_environment_variable.py +++ b/launch/launch/actions/append_environment_variable.py @@ -17,7 +17,6 @@ import os from typing import List from typing import Union -from typing import cast from ..action import Action from ..frontend import Entity @@ -76,14 +75,14 @@ def parse( ): """Parse an 'append_env' entity.""" _, kwargs = super().parse(entity, parser) - kwargs['name'] = parser.parse_substitution(cast(str, entity.get_attr('name'))) - kwargs['value'] = parser.parse_substitution(cast(str, entity.get_attr('value'))) + kwargs['name'] = parser.parse_substitution(entity.get_attr('name')) + kwargs['value'] = parser.parse_substitution(entity.get_attr('value')) prepend = entity.get_attr('prepend', optional=True, data_type=bool, can_be_str=True) if prepend is not None: - kwargs['prepend'] = parser.parse_if_substitutions(cast(Union[str, bool], prepend)) + kwargs['prepend'] = parser.parse_if_substitutions(prepend) separator = entity.get_attr('separator', optional=True) if separator is not None: - kwargs['separator'] = parser.parse_substitution(cast(str, separator)) + kwargs['separator'] = parser.parse_substitution(separator) return cls, kwargs @property diff --git a/launch/launch/actions/declare_launch_argument.py b/launch/launch/actions/declare_launch_argument.py index 54aa2fd13..5290bc30c 100644 --- a/launch/launch/actions/declare_launch_argument.py +++ b/launch/launch/actions/declare_launch_argument.py @@ -17,7 +17,6 @@ from typing import List from typing import Optional from typing import Text -from typing import cast import launch.logging @@ -166,19 +165,17 @@ def parse( ): """Parse `arg` tag.""" _, kwargs = super().parse(entity, parser) - kwargs['name'] = parser.escape_characters(cast(str, entity.get_attr('name'))) + kwargs['name'] = parser.escape_characters(entity.get_attr('name')) default_value = entity.get_attr('default', optional=True) if default_value is not None: - kwargs['default_value'] = parser.parse_substitution(cast(str, default_value)) + kwargs['default_value'] = parser.parse_substitution(default_value) description = entity.get_attr('description', optional=True) if description is not None: - kwargs['description'] = parser.escape_characters(cast(str, description)) - # TODO: What to do here? get_attr is supposed to parse scalar / list of scalars, but in - # this case asks for a list of entities? - choices = cast(List[Entity], entity.get_attr('choice', data_type=List[Entity], optional=True)) # type: ignore + kwargs['description'] = parser.escape_characters(description) + choices = entity.get_attr('choice', data_type=List[Entity], optional=True) if choices is not None: kwargs['choices'] = [ - parser.escape_characters(cast(str, choice.get_attr('value'))) for choice in choices + parser.escape_characters(choice.get_attr('value')) for choice in choices ] return cls, kwargs diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 913b8cafb..03bdc3348 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -318,35 +318,35 @@ def parse( ignore = [] if 'cmd' not in ignore: - kwargs['cmd'] = cls._parse_cmdline(cast(str, entity.get_attr('cmd')), parser) + kwargs['cmd'] = cls._parse_cmdline(entity.get_attr('cmd'), parser) if 'cwd' not in ignore: - cwd = cast(Optional[str], entity.get_attr('cwd', optional=True)) + cwd = entity.get_attr('cwd', optional=True) if cwd is not None: kwargs['cwd'] = parser.parse_substitution(cwd) if 'name' not in ignore: - name = cast(Optional[str], entity.get_attr('name', optional=True)) + name = entity.get_attr('name', optional=True) if name is not None: kwargs['name'] = parser.parse_substitution(name) if 'prefix' not in ignore: - prefix = cast(Optional[str], entity.get_attr('launch-prefix', optional=True)) + prefix = entity.get_attr('launch-prefix', optional=True) if prefix is not None: kwargs['prefix'] = parser.parse_substitution(prefix) if 'output' not in ignore: - output = cast(Optional[str], entity.get_attr('output', optional=True)) + output = entity.get_attr('output', optional=True) if output is not None: kwargs['output'] = parser.parse_substitution(output) if 'respawn' not in ignore: - respawn = cast(Optional[str], entity.get_attr('respawn', optional=True)) + respawn = entity.get_attr('respawn', optional=True) if respawn is not None: kwargs['respawn'] = parser.parse_substitution(respawn) if 'respawn_delay' not in ignore: - respawn_delay = cast(Optional[float], entity.get_attr('respawn_delay', data_type=float, optional=True)) + respawn_delay = entity.get_attr('respawn_delay', data_type=float, optional=True) if respawn_delay is not None: if respawn_delay < 0.0: raise ValueError( @@ -356,7 +356,7 @@ def parse( kwargs['respawn_delay'] = respawn_delay if 'sigkill_timeout' not in ignore: - sigkill_timeout = cast(Optional[float], entity.get_attr('sigkill_timeout', data_type=float, optional=True)) + sigkill_timeout = entity.get_attr('sigkill_timeout', data_type=float, optional=True) if sigkill_timeout is not None: if sigkill_timeout < 0.0: raise ValueError( @@ -366,7 +366,7 @@ def parse( kwargs['sigkill_timeout'] = str(sigkill_timeout) if 'sigterm_timeout' not in ignore: - sigterm_timeout = cast(Optional[float], entity.get_attr('sigterm_timeout', data_type=float, optional=True)) + sigterm_timeout = entity.get_attr('sigterm_timeout', data_type=float, optional=True) if sigterm_timeout is not None: if sigterm_timeout < 0.0: raise ValueError( @@ -376,12 +376,12 @@ def parse( kwargs['sigterm_timeout'] = str(sigterm_timeout) if 'shell' not in ignore: - shell = cast(Optional[bool], entity.get_attr('shell', data_type=bool, optional=True)) + shell = entity.get_attr('shell', data_type=bool, optional=True) if shell is not None: kwargs['shell'] = shell if 'emulate_tty' not in ignore: - emulate_tty = cast(Optional[bool], entity.get_attr('emulate_tty', data_type=bool, optional=True)) + emulate_tty = entity.get_attr('emulate_tty', data_type=bool, optional=True) if emulate_tty is not None: kwargs['emulate_tty'] = emulate_tty @@ -389,12 +389,11 @@ def parse( # Conditions won't be allowed in the `env` tag. # If that feature is needed, `set_enviroment_variable` and # `unset_enviroment_variable` actions should be used. - # TODO: Fixup the data_type annotation - env = cast(Optional[List[Entity]], entity.get_attr('env', data_type=List[Entity], optional=True)) # type: ignore + env = entity.get_attr('env', data_type=List[Entity], optional=True) if env is not None: kwargs['additional_env'] = { - tuple(parser.parse_substitution(cast(str, e.get_attr('name')))): - parser.parse_substitution(cast(str, e.get_attr('value'))) for e in env + tuple(parser.parse_substitution(e.get_attr('name'))): + parser.parse_substitution(e.get_attr('value')) for e in env } for e in env: e.assert_entity_completely_parsed() diff --git a/launch/launch/frontend/entity.py b/launch/launch/frontend/entity.py index 53d417850..cbf468c4a 100644 --- a/launch/launch/frontend/entity.py +++ b/launch/launch/frontend/entity.py @@ -15,14 +15,20 @@ """Module for Entity class.""" from typing import List +from typing import Literal from typing import Optional from typing import Text +from typing import Type from typing import Union +from typing import TypeVar +from typing import overload from launch.utilities.type_utils import AllowedTypesType from launch.utilities.type_utils import AllowedValueType +TargetType = TypeVar("TargetType") + class Entity: """Single item in the intermediate front_end representation.""" @@ -41,17 +47,46 @@ def children(self) -> List['Entity']: """Get the Entity's children.""" raise NotImplementedError() + # 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], optional: Literal[True], can_be_str: bool = True) -> Optional[TargetType]: + ... + + @overload + def get_attr(self, name: Text, *, data_type: Type[TargetType], can_be_str: bool = True) -> TargetType: + ... + + @overload + def get_attr(self, name: Text, *, optional: Literal[False], can_be_str: bool = True) -> str: + ... + + @overload + def get_attr(self, name: Text, *, optional: Literal[True], can_be_str: bool = True) -> Optional[str]: + ... + + @overload + def get_attr(self, name: Text, *, can_be_str: bool = True) -> str: + ... + + def get_attr( 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. From 63af13fc7aae6a8a90388173fd3069f33af2f789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 6 Jan 2023 15:50:16 +0900 Subject: [PATCH 09/53] Minor fixes in include launch description --- launch/launch/actions/include_launch_description.py | 2 +- launch/launch/descriptions/executable.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/launch/launch/actions/include_launch_description.py b/launch/launch/actions/include_launch_description.py index d022d5ee5..2d3395d52 100644 --- a/launch/launch/actions/include_launch_description.py +++ b/launch/launch/actions/include_launch_description.py @@ -78,7 +78,7 @@ def __init__( if not isinstance(launch_description_source, LaunchDescriptionSource): launch_description_source = AnyLaunchDescriptionSource(launch_description_source) self.__launch_description_source = launch_description_source - self.__launch_arguments = () if launch_arguments is None else tuple(launch_arguments) + self.__launch_arguments = tuple() if launch_arguments is None else tuple(launch_arguments) self.__logger = launch.logging.get_logger(__name__) @classmethod diff --git a/launch/launch/descriptions/executable.py b/launch/launch/descriptions/executable.py index 109d1fc6d..c48d19a41 100644 --- a/launch/launch/descriptions/executable.py +++ b/launch/launch/descriptions/executable.py @@ -101,10 +101,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 +178,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 From f588667aa4a1146773b0f313d51e66135229e322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 6 Jan 2023 15:50:49 +0900 Subject: [PATCH 10/53] More small fixes - Event handlers - Boolean substitutions --- .../launch/event_handlers/on_process_exit.py | 14 +++------- launch/launch/event_handlers/on_process_io.py | 2 +- .../launch/event_handlers/on_process_start.py | 2 +- launch/launch/launch_context.py | 2 +- launch/launch/substitutions/anon_name.py | 4 +-- .../substitutions/boolean_substitution.py | 26 ++++++++++--------- .../launch/substitutions/this_launch_file.py | 4 +-- 7 files changed, 24 insertions(+), 30 deletions(-) diff --git a/launch/launch/event_handlers/on_process_exit.py b/launch/launch/event_handlers/on_process_exit.py index c037e21a4..6681dcf35 100644 --- a/launch/launch/event_handlers/on_process_exit.py +++ b/launch/launch/event_handlers/on_process_exit.py @@ -28,7 +28,7 @@ if TYPE_CHECKING: - from ..actions import Action # noqa: F401 + from ..action import Action # noqa: F401 from ..actions import ExecuteLocal # noqa: F401 @@ -54,17 +54,9 @@ def __init__( ) -> 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[SomeActionsType, Callable[[Event, LaunchContext], Optional[SomeActionsType]]], 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..293586825 100644 --- a/launch/launch/event_handlers/on_process_io.py +++ b/launch/launch/event_handlers/on_process_io.py @@ -27,7 +27,7 @@ from ..some_actions_type import SomeActionsType if TYPE_CHECKING: - from ..actions import Action # noqa: F401 + from ..action import Action # noqa: F401 from ..actions import ExecuteLocal # noqa: F401 diff --git a/launch/launch/event_handlers/on_process_start.py b/launch/launch/event_handlers/on_process_start.py index 158f4a974..fe30a1182 100644 --- a/launch/launch/event_handlers/on_process_start.py +++ b/launch/launch/event_handlers/on_process_start.py @@ -27,7 +27,7 @@ from ..some_actions_type import SomeActionsType if TYPE_CHECKING: - from ..actions import Action # noqa: F401 + from ..action import Action # noqa: F401 from ..actions import ExecuteProcess # noqa: F401 diff --git a/launch/launch/launch_context.py b/launch/launch/launch_context.py index 498b8a7de..26c4af00e 100644 --- a/launch/launch/launch_context.py +++ b/launch/launch/launch_context.py @@ -201,7 +201,7 @@ def launch_configurations(self) -> Dict[Text, Text]: return self.__launch_configurations @property - def environment(self) -> Mapping[Text, Text]: + def environment(self) -> Dict[Text, Text]: """Getter for environment variables dictionary.""" return os.environ diff --git a/launch/launch/substitutions/anon_name.py b/launch/launch/substitutions/anon_name.py index 09bf92d3f..8fe9522f5 100644 --- a/launch/launch/substitutions/anon_name.py +++ b/launch/launch/substitutions/anon_name.py @@ -14,7 +14,7 @@ """Module for the anonymous name substitution.""" -from typing import Iterable +from typing import Sequence from typing import List from typing import Text @@ -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') 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/this_launch_file.py b/launch/launch/substitutions/this_launch_file.py index a0744c12a..cdc499303 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,7 +33,7 @@ def __init__(self) -> None: super().__init__() @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `ThisLaunchFile` substitution.""" if len(data) != 0: raise TypeError("filename substitution doesn't expect arguments") From 239858e1b4e5cde55eccd36251448be876ebcedb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 09:42:13 +0900 Subject: [PATCH 11/53] Fixup minor errors in launch config --- launch/launch/substitutions/launch_configuration.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/launch/launch/substitutions/launch_configuration.py b/launch/launch/substitutions/launch_configuration.py index 41fdf80fa..483bbd475 100644 --- a/launch/launch/substitutions/launch_configuration.py +++ b/launch/launch/substitutions/launch_configuration.py @@ -21,6 +21,7 @@ from typing import Optional from typing import Text from typing import Union +from typing import Sequence from .substitution_failure import SubstitutionFailure from ..frontend import expose_substitution @@ -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') From a8446cb843588e51e22467ab58084550e9cbbf38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 09:47:40 +0900 Subject: [PATCH 12/53] Rename some_actions_type -> some_entities_type --- launch/launch/__init__.py | 8 ++++---- launch/launch/actions/execute_local.py | 10 +++++----- launch/launch/actions/opaque_coroutine.py | 4 ++-- launch/launch/actions/timer_action.py | 4 ++-- launch/launch/event_handler.py | 12 ++++++------ launch/launch/event_handlers/on_action_event_base.py | 8 ++++---- .../launch/event_handlers/on_execution_complete.py | 10 +++++----- launch/launch/event_handlers/on_process_exit.py | 8 ++++---- launch/launch/event_handlers/on_process_io.py | 12 ++++++------ launch/launch/event_handlers/on_process_start.py | 10 +++++----- launch/launch/event_handlers/on_shutdown.py | 8 ++++---- launch/launch/launch_service.py | 4 ++-- .../{some_actions_type.py => some_entities_type.py} | 7 +++---- launch/test/launch/test_event_handler.py | 4 ++-- launch_testing/launch_testing/actions/test.py | 4 ++-- .../event_handlers/stdout_ready_listener.py | 6 +++--- 16 files changed, 59 insertions(+), 60 deletions(-) rename launch/launch/{some_actions_type.py => some_entities_type.py} (89%) 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/actions/execute_local.py b/launch/launch/actions/execute_local.py index 2258e3140..76ad73339 100644 --- a/launch/launch/actions/execute_local.py +++ b/launch/launch/actions/execute_local.py @@ -62,7 +62,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 @@ -93,8 +93,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, @@ -347,7 +347,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)), ) @@ -434,7 +434,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, diff --git a/launch/launch/actions/opaque_coroutine.py b/launch/launch/actions/opaque_coroutine.py index 0ebbd037a..0177257f3 100644 --- a/launch/launch/actions/opaque_coroutine.py +++ b/launch/launch/actions/opaque_coroutine.py @@ -29,7 +29,7 @@ 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 @@ -92,7 +92,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/timer_action.py b/launch/launch/actions/timer_action.py index 66d989e1f..e5cd473b1 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 diff --git a/launch/launch/event_handler.py b/launch/launch/event_handler.py index bc98f6ae6..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. @@ -107,7 +107,7 @@ def __init__( self, *, matcher: Callable[[Event], bool], - entities: Optional[SomeActionsType] = None, + entities: Optional[SomeEntitiesType] = None, handle_once: bool = False ) -> None: """ @@ -129,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 3cc37c509..e97527736 100644 --- a/launch/launch/event_handlers/on_action_event_base.py +++ b/launch/launch/event_handlers/on_action_event_base.py @@ -27,7 +27,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 @@ -41,8 +41,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'], @@ -102,7 +102,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) 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 6681dcf35..a121be50c 100644 --- a/launch/launch/event_handlers/on_process_exit.py +++ b/launch/launch/event_handlers/on_process_exit.py @@ -24,7 +24,7 @@ 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: @@ -47,8 +47,8 @@ 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: @@ -56,7 +56,7 @@ def __init__( from ..actions import ExecuteLocal # noqa: F811 super().__init__( action_matcher=cast(Optional[Union[Callable[['Action'], bool], 'Action']], target_action), - on_event=cast(Union[SomeActionsType, Callable[[Event, LaunchContext], Optional[SomeActionsType]]], on_exit), + 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 293586825..a20072cd6 100644 --- a/launch/launch/event_handlers/on_process_io.py +++ b/launch/launch/event_handlers/on_process_io.py @@ -24,7 +24,7 @@ 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 ..action import Action # noqa: F401 @@ -35,15 +35,15 @@ 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, + 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.""" @@ -52,7 +52,7 @@ def __init__( 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 fe30a1182..06bfe777a 100644 --- a/launch/launch/event_handlers/on_process_start.py +++ b/launch/launch/event_handlers/on_process_start.py @@ -24,7 +24,7 @@ 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 ..action import Action # noqa: F401 @@ -46,8 +46,8 @@ def __init__( Optional[Union[Callable[['ExecuteProcess'], bool], 'ExecuteProcess']] = None, on_start: Union[ - SomeActionsType, - Callable[[ProcessStarted, LaunchContext], Optional[SomeActionsType]]], + SomeEntitiesType, + Callable[[ProcessStarted, LaunchContext], Optional[SomeEntitiesType]]], **kwargs ) -> None: """Create an OnProcessStart event handler.""" @@ -57,8 +57,8 @@ def __init__( 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..8820ba5c2 100644 --- a/launch/launch/event_handlers/on_shutdown.py +++ b/launch/launch/event_handlers/on_shutdown.py @@ -24,7 +24,7 @@ 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: @@ -37,8 +37,8 @@ class OnShutdown(BaseEventHandler): 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.""" @@ -52,7 +52,7 @@ def __init__( if not callable(on_shutdown): self.__on_shutdown = (lambda event, context: 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/launch_service.py b/launch/launch/launch_service.py index 3dd1e7429..412b9b473 100644 --- a/launch/launch/launch_service.py +++ b/launch/launch/launch_service.py @@ -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 diff --git a/launch/launch/some_actions_type.py b/launch/launch/some_entities_type.py similarity index 89% rename from launch/launch/some_actions_type.py rename to launch/launch/some_entities_type.py index 03d819584..b42480184 100644 --- a/launch/launch/some_actions_type.py +++ b/launch/launch/some_entities_type.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Module for SomeActionsType type.""" +"""Module for SomeEntitiesType type.""" import collections.abc from typing import Iterable @@ -20,13 +20,12 @@ from .launch_description_entity import LaunchDescriptionEntity - -SomeActionsType = Union[ +SomeEntitiesType = Union[ LaunchDescriptionEntity, Iterable[LaunchDescriptionEntity], ] -SomeActionsType_types_tuple = ( +SomeEntitiesType_types_tuple = ( LaunchDescriptionEntity, collections.abc.Iterable, ) 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_testing/launch_testing/actions/test.py b/launch_testing/launch_testing/actions/test.py index f77d8c25d..2e72eea08 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,7 @@ 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 ( From e4b8edc920789377eb8cd1a123fcb4b25f27e710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 10:06:06 +0900 Subject: [PATCH 13/53] Rename coroutine -> coroutine_function Do not change public API just yet --- launch/launch/actions/opaque_coroutine.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/launch/launch/actions/opaque_coroutine.py b/launch/launch/actions/opaque_coroutine.py index 0177257f3..98491bac8 100644 --- a/launch/launch/actions/opaque_coroutine.py +++ b/launch/launch/actions/opaque_coroutine.py @@ -17,12 +17,13 @@ import asyncio import collections.abc from typing import Any -from typing import Coroutine from typing import Dict from typing import Iterable from typing import List from typing import Optional from typing import Text +from typing import Callable +from typing import Awaitable from ..action import Action from ..event import Event @@ -35,13 +36,13 @@ 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) ) ) From dfb16539e7c63b5ccfee35a59c978b44b74749a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 10:06:27 +0900 Subject: [PATCH 14/53] Minor typing mistake --- launch/launch/launch_context.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch/launch/launch_context.py b/launch/launch/launch_context.py index 26c4af00e..498b8a7de 100644 --- a/launch/launch/launch_context.py +++ b/launch/launch/launch_context.py @@ -201,7 +201,7 @@ def launch_configurations(self) -> Dict[Text, Text]: return self.__launch_configurations @property - def environment(self) -> Dict[Text, Text]: + def environment(self) -> Mapping[Text, Text]: """Getter for environment variables dictionary.""" return os.environ From bd37fdceb5a036f295c150e43ab86a673d926026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 10:31:53 +0900 Subject: [PATCH 15/53] Fix typing of the handlers --- launch/launch/logging/handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launch/launch/logging/handlers.py b/launch/launch/logging/handlers.py index 454f296a1..b93f4c710 100644 --- a/launch/launch/logging/handlers.py +++ b/launch/launch/logging/handlers.py @@ -15,7 +15,7 @@ """Module with handlers for launch specific logging.""" import sys - +import types def with_per_logger_formatting(cls): """Add per logger formatting capabilities to the given logging.Handler.""" @@ -50,7 +50,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): From fc271c6bfe481c86b2a9c5482cae2a81e09ce25a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 10:48:18 +0900 Subject: [PATCH 16/53] Minor fixes --- launch/launch/actions/register_event_handler.py | 6 +++--- launch/launch/conditions/launch_configuration_equals.py | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/launch/launch/actions/register_event_handler.py b/launch/launch/actions/register_event_handler.py index 85272fb75..6b6cb853b 100644 --- a/launch/launch/actions/register_event_handler.py +++ b/launch/launch/actions/register_event_handler.py @@ -14,7 +14,6 @@ """Module for the RegisterEventHandler action.""" -from typing import Iterable from typing import List from typing import Text from typing import Tuple @@ -23,6 +22,7 @@ from ..event_handler import BaseEventHandler from ..launch_context import LaunchContext from ..launch_description_entity import LaunchDescriptionEntity +from ..utilities import normalize_to_list_of_substitutions class RegisterEventHandler(Action): @@ -54,9 +54,9 @@ def execute(self, context: LaunchContext): def describe_conditional_sub_entities(self) -> List[Tuple[ Text, # text description of the condition - Iterable[LaunchDescriptionEntity], # list of conditional sub-entities + List[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_substitutions(event_handler_description[1])) ] if event_handler_description[1] else [] diff --git a/launch/launch/conditions/launch_configuration_equals.py b/launch/launch/conditions/launch_configuration_equals.py index 3a41fee03..8439df7a2 100644 --- a/launch/launch/conditions/launch_configuration_equals.py +++ b/launch/launch/conditions/launch_configuration_equals.py @@ -16,11 +16,13 @@ from typing import Optional from typing import Text +from typing import List import warnings 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 @@ -59,6 +61,7 @@ def __init__( ) 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: From 7ffa5e359caa525e37604c6ac44ec45e4985d810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 10:50:01 +0900 Subject: [PATCH 17/53] Another Sequence fix --- launch/launch/substitutions/this_launch_file_dir.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launch/launch/substitutions/this_launch_file_dir.py b/launch/launch/substitutions/this_launch_file_dir.py index 59b7464e0..9784f1497 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,7 +33,7 @@ def __init__(self) -> None: super().__init__() @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + def parse(cls, data: Sequence[SomeSubstitutionsType]): """Parse `ThisLaunchFileDir` substitution.""" if len(data) != 0: raise TypeError("dirname substitution doesn't expect arguments") From c30e0a3a012839a3a0ecf9184e1e25c727c9c5a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 11:21:35 +0900 Subject: [PATCH 18/53] Some fixes to logging --- launch/launch/actions/execute_local.py | 2 +- launch/launch/actions/timer_action.py | 2 +- launch/launch/logging/__init__.py | 10 ++++++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/launch/launch/actions/execute_local.py b/launch/launch/actions/execute_local.py index 76ad73339..91acd0fac 100644 --- a/launch/launch/actions/execute_local.py +++ b/launch/launch/actions/execute_local.py @@ -33,7 +33,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 diff --git a/launch/launch/actions/timer_action.py b/launch/launch/actions/timer_action.py index e5cd473b1..7d470dcf8 100644 --- a/launch/launch/actions/timer_action.py +++ b/launch/launch/actions/timer_action.py @@ -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/logging/__init__.py b/launch/launch/logging/__init__.py index 988de8049..35c172656 100644 --- a/launch/launch/logging/__init__.py +++ b/launch/launch/logging/__init__.py @@ -25,6 +25,7 @@ import sys from typing import List +from typing import Any from . import handlers @@ -288,7 +289,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() @@ -456,8 +457,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): From 83e9507e885ecb171251a4e429b2985430f1cad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 11:21:57 +0900 Subject: [PATCH 19/53] Many Iterable -> Sequence fixes --- launch/launch/substitutions/command.py | 4 ++-- launch/launch/substitutions/environment_variable.py | 4 ++-- launch/launch/substitutions/equals_substitution.py | 3 ++- launch/launch/substitutions/find_executable.py | 6 +++--- launch/launch/substitutions/launch_log_dir.py | 4 ++-- launch/launch/substitutions/python_expression.py | 4 ++-- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/launch/launch/substitutions/command.py b/launch/launch/substitutions/command.py index e918937ef..7a6c055da 100644 --- a/launch/launch/substitutions/command.py +++ b/launch/launch/substitutions/command.py @@ -17,7 +17,7 @@ import os import shlex import subprocess -from typing import Iterable +from typing import Sequence from typing import List from typing import Text @@ -65,7 +65,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') diff --git a/launch/launch/substitutions/environment_variable.py b/launch/launch/substitutions/environment_variable.py index a6606b96d..1f7d8c520 100644 --- a/launch/launch/substitutions/environment_variable.py +++ b/launch/launch/substitutions/environment_variable.py @@ -14,7 +14,7 @@ """Module for the EnvironmentVariable substitution.""" -from typing import Iterable +from typing import Sequence from typing import List from typing import Optional from typing import Text @@ -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') diff --git a/launch/launch/substitutions/equals_substitution.py b/launch/launch/substitutions/equals_substitution.py index 0fb0b9f2f..2718e9a7a 100644 --- a/launch/launch/substitutions/equals_substitution.py +++ b/launch/launch/substitutions/equals_substitution.py @@ -17,6 +17,7 @@ import math from typing import Any +from typing import Sequence from typing import Iterable from typing import Optional from typing import Text @@ -83,7 +84,7 @@ def __init__( self.__right = normalize_to_list_of_substitutions(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') diff --git a/launch/launch/substitutions/find_executable.py b/launch/launch/substitutions/find_executable.py index 56bac7466..3177583a3 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 Sequence from typing import List 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_log_dir.py b/launch/launch/substitutions/launch_log_dir.py index 37867d1ed..dbe29847d 100644 --- a/launch/launch/substitutions/launch_log_dir.py +++ b/launch/launch/substitutions/launch_log_dir.py @@ -14,7 +14,7 @@ """Module for the LaunchLogDir substitution.""" -from typing import Iterable +from typing import Sequence from typing import Text from ..frontend.expose import expose_substitution @@ -34,7 +34,7 @@ def __init__(self) -> None: super().__init__() @classmethod - def parse(cls, data: Iterable[SomeSubstitutionsType]): + 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") diff --git a/launch/launch/substitutions/python_expression.py b/launch/launch/substitutions/python_expression.py index 3ee4fe957..e8efaef34 100644 --- a/launch/launch/substitutions/python_expression.py +++ b/launch/launch/substitutions/python_expression.py @@ -16,7 +16,7 @@ import collections.abc import math -from typing import Iterable +from typing import Sequence from typing import List from typing import Text @@ -51,7 +51,7 @@ 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.""" if len(data) != 1: raise TypeError('eval substitution expects 1 argument') From a33ed8e7c3d0e96273c8096bbdcd8e803ca99624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 11:24:43 +0900 Subject: [PATCH 20/53] Fixup launch_service --- launch/launch/launch_service.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/launch/launch/launch_service.py b/launch/launch/launch_service.py index 412b9b473..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 @@ -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): From 87f669e748f7aaee94bc08b07e09b6c6c53ae5d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 13:47:14 +0900 Subject: [PATCH 21/53] Improve equals_substitution --- .../substitutions/equals_substitution.py | 18 ++++++++++++++---- launch/launch/utilities/type_utils.py | 4 ++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/launch/launch/substitutions/equals_substitution.py b/launch/launch/substitutions/equals_substitution.py index 2718e9a7a..ca5cf50f0 100644 --- a/launch/launch/substitutions/equals_substitution.py +++ b/launch/launch/substitutions/equals_substitution.py @@ -19,9 +19,11 @@ from typing import Any from typing import Sequence from typing import Iterable +from typing import List from typing import Optional from typing import Text from typing import Union +from typing import cast from ..frontend import expose_substitution from ..launch_context import LaunchContext @@ -80,8 +82,16 @@ 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: Sequence[SomeSubstitutionsType]): @@ -91,12 +101,12 @@ def parse(cls, data: Sequence[SomeSubstitutionsType]): 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/utilities/type_utils.py b/launch/launch/utilities/type_utils.py index 69b87eab1..8941a2910 100644 --- a/launch/launch/utilities/type_utils.py +++ b/launch/launch/utilities/type_utils.py @@ -348,6 +348,10 @@ def get_typed_value( 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. From 3c153fca4e57ce1f26789134b55032fefee2f45d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 13:49:10 +0900 Subject: [PATCH 22/53] Fixup environment variable --- launch/launch/substitutions/environment_variable.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/launch/launch/substitutions/environment_variable.py b/launch/launch/substitutions/environment_variable.py index 1f7d8c520..7f13bf94e 100644 --- a/launch/launch/substitutions/environment_variable.py +++ b/launch/launch/substitutions/environment_variable.py @@ -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( From 7433476df81a50154e1da884846203111e28443f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 14:00:32 +0900 Subject: [PATCH 23/53] Fix some execute process / execute local things --- launch/launch/events/process/process_targeted_event.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/launch/launch/events/process/process_targeted_event.py b/launch/launch/events/process/process_targeted_event.py index 0ef27ba6d..ea94e032d 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 ExecuteLocal # noqa: F401 + from ...actions import ExecuteProcess # noqa: F401 class ProcessTargetedEvent(Event): @@ -28,7 +28,7 @@ class ProcessTargetedEvent(Event): name = 'launch.events.process.ProcessTargetedEvent' - def __init__(self, *, process_matcher: Callable[['ExecuteLocal'], bool]) -> None: + def __init__(self, *, process_matcher: Callable[['ExecuteProcess'], bool]) -> None: """ Create a ProcessTargetedEvent. @@ -46,6 +46,6 @@ def __init__(self, *, process_matcher: Callable[['ExecuteLocal'], bool]) -> None self.__process_matcher = process_matcher @property - def process_matcher(self) -> Callable[['ExecuteLocal'], bool]: + def process_matcher(self) -> Callable[['ExecuteProcess'], bool]: """Getter for process_matcher.""" return self.__process_matcher From 77a73df1d7a3f63e53004736f0a5b79911436e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 14:00:46 +0900 Subject: [PATCH 24/53] Fixup anon name It used to store nested mappings in launch_configurations, but this is not permitted by the API, which specifies that launch_configurations is a flat mapping of str -> str --- launch/launch/substitutions/anon_name.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/launch/launch/substitutions/anon_name.py b/launch/launch/substitutions/anon_name.py index 8fe9522f5..6bb79e1f9 100644 --- a/launch/launch/substitutions/anon_name.py +++ b/launch/launch/substitutions/anon_name.py @@ -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.""" From 085f43f9e979a3804026b7073d98a6f4cc23c027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 14:01:57 +0900 Subject: [PATCH 25/53] Fixup command Simply needed some Union --- launch/launch/substitutions/command.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/launch/launch/substitutions/command.py b/launch/launch/substitutions/command.py index 7a6c055da..86d71e019 100644 --- a/launch/launch/substitutions/command.py +++ b/launch/launch/substitutions/command.py @@ -20,6 +20,7 @@ from typing import Sequence from typing import List from typing import Text +from typing import Union import launch.logging @@ -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: From 1cdb863164168921fdbcf6cc31e3b94d21fc56e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 17:14:24 +0900 Subject: [PATCH 26/53] Fix the source location annotation --- launch/launch/launch_description_source.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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__) From f7571b0075a25c782828112a49ebd023ca7a5329 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 17:46:49 +0900 Subject: [PATCH 27/53] More type errors... --- launch/launch/event_handlers/on_shutdown.py | 13 ++++++++++--- launch/launch/launch_introspector.py | 12 +++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/launch/launch/event_handlers/on_shutdown.py b/launch/launch/event_handlers/on_shutdown.py index 8820ba5c2..76ef4d1d2 100644 --- a/launch/launch/event_handlers/on_shutdown.py +++ b/launch/launch/event_handlers/on_shutdown.py @@ -31,6 +31,12 @@ 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.""" @@ -48,9 +54,10 @@ 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[SomeEntitiesType]: """Handle the given event.""" diff --git a/launch/launch/launch_introspector.py b/launch/launch/launch_introspector.py index 41cb34ca8..78694ed93 100644 --- a/launch/launch/launch_introspector.py +++ b/launch/launch/launch_introspector.py @@ -16,6 +16,7 @@ from typing import cast from typing import List +from typing import Iterable from typing import Text from .action import Action @@ -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,14 @@ 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)))] From d840ae6e2e0ec0fdf41b3d1ea8b888b665f8977c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 18:16:03 +0900 Subject: [PATCH 28/53] Fix some more ExecuteLocal weirdness --- launch/launch/event_handlers/on_process_io.py | 5 +---- launch/launch/events/process/process_targeted_event.py | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/launch/launch/event_handlers/on_process_io.py b/launch/launch/event_handlers/on_process_io.py index a20072cd6..66d79dc4b 100644 --- a/launch/launch/event_handlers/on_process_io.py +++ b/launch/launch/event_handlers/on_process_io.py @@ -40,7 +40,7 @@ def __init__( self, *, target_action: - Optional[Union[Callable[['ExecuteLocal'], bool], 'ExecuteLocal']] = 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, @@ -48,9 +48,6 @@ def __init__( ) -> 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[SomeEntitiesType]: event = cast(ProcessIO, event) diff --git a/launch/launch/events/process/process_targeted_event.py b/launch/launch/events/process/process_targeted_event.py index ea94e032d..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. @@ -46,6 +46,6 @@ def __init__(self, *, process_matcher: Callable[['ExecuteProcess'], bool]) -> No 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 From d05fd8603f66dac50d847028ff9cee35e6057667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 18:16:17 +0900 Subject: [PATCH 29/53] Fix the type of launch file loaders --- .../launch_description_sources/any_launch_file_utilities.py | 6 +++++- .../frontend_launch_file_utilities.py | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) 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) From 994201fdd29ac1ae84ee0f195ad7e2ad48885148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 18:27:33 +0900 Subject: [PATCH 30/53] Some people need to modify the environment --- launch/launch/launch_context.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 From b2676e809c93b513b1402daa24476965e930cbd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 18:58:02 +0900 Subject: [PATCH 31/53] Fixup register_event_handler Needed a new normalization thing. I don't know how it worked before... --- .../launch/actions/register_event_handler.py | 8 +++-- launch/launch/utilities/__init__.py | 2 ++ .../normalize_to_list_of_entities_impl.py | 33 +++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 launch/launch/utilities/normalize_to_list_of_entities_impl.py diff --git a/launch/launch/actions/register_event_handler.py b/launch/launch/actions/register_event_handler.py index 6b6cb853b..30913f785 100644 --- a/launch/launch/actions/register_event_handler.py +++ b/launch/launch/actions/register_event_handler.py @@ -14,6 +14,7 @@ """Module for the RegisterEventHandler action.""" +from typing import Iterable from typing import List from typing import Text from typing import Tuple @@ -22,7 +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_substitutions +from ..utilities import normalize_to_list_of_entities class RegisterEventHandler(Action): @@ -54,9 +55,10 @@ def execute(self, context: LaunchContext): def describe_conditional_sub_entities(self) -> List[Tuple[ Text, # text description of the condition - List[LaunchDescriptionEntity], # list of conditional sub-entities + Iterable[LaunchDescriptionEntity], # list of conditional sub-entities ]]: event_handler_description = self.__event_handler.describe() + return [ - (event_handler_description[0], normalize_to_list_of_substitutions(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/utilities/__init__.py b/launch/launch/utilities/__init__.py index f3efb4de5..bd0908ab2 100644 --- a/launch/launch/utilities/__init__.py +++ b/launch/launch/utilities/__init__.py @@ -18,6 +18,7 @@ from .create_future_impl import create_future from .ensure_argument_type_impl import ensure_argument_type from .normalize_to_list_of_substitutions_impl import normalize_to_list_of_substitutions +from .normalize_to_list_of_entities_impl import normalize_to_list_of_entities from .perform_substitutions_impl import perform_substitutions from .signal_management import AsyncSafeSignalManager from .visit_all_entities_and_collect_futures_impl import visit_all_entities_and_collect_futures @@ -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..cb4db7b87 --- /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 ..some_entities_type import SomeEntitiesType +from ..launch_description_entity import LaunchDescriptionEntity + + +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 From 6031c2cb0198132f6b5abcc215137a49057d25bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 16 Feb 2023 19:00:44 +0900 Subject: [PATCH 32/53] Remove superfluous cast --- launch/launch/event_handlers/on_process_start.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/launch/launch/event_handlers/on_process_start.py b/launch/launch/event_handlers/on_process_start.py index 06bfe777a..0c1f2304d 100644 --- a/launch/launch/event_handlers/on_process_start.py +++ b/launch/launch/event_handlers/on_process_start.py @@ -43,7 +43,7 @@ def __init__( self, *, target_action: - Optional[Union[Callable[['ExecuteProcess'], bool], 'ExecuteProcess']] = None, + Optional[Union[Callable[['Action'], bool], 'Action']] = None, on_start: Union[ SomeEntitiesType, @@ -52,9 +52,6 @@ def __init__( ) -> 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[ SomeEntitiesType, From b258111ccf17ef23f563395a451b772648711ce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 09:09:01 +0900 Subject: [PATCH 33/53] Minor signal process fix --- launch/launch/events/process/signal_process.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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. From 43b1e9724a8303ae642be7effd0b77379dbd3be2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 09:14:16 +0900 Subject: [PATCH 34/53] Replace try-catch with version switch --- launch/launch/frontend/parser.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/launch/launch/frontend/parser.py b/launch/launch/frontend/parser.py index 2bfea7d98..f9c8ce896 100644 --- a/launch/launch/frontend/parser.py +++ b/launch/launch/frontend/parser.py @@ -27,10 +27,11 @@ from typing import TYPE_CHECKING from typing import Union import warnings +import sys -try: +if sys.version_info >= (3, 8): import importlib.metadata as importlib_metadata -except ModuleNotFoundError: +else: import importlib_metadata from .entity import Entity From c38cb843b2a7207bffbae04f1932c1fdb0011e7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 09:27:14 +0900 Subject: [PATCH 35/53] Last touches to mypy --- launch/launch/descriptions/executable.py | 2 +- launch/launch/frontend/expose.py | 6 ++++-- launch/launch/frontend/parser.py | 17 +++++++++++++---- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/launch/launch/descriptions/executable.py b/launch/launch/descriptions/executable.py index c48d19a41..1c121f232 100644 --- a/launch/launch/descriptions/executable.py +++ b/launch/launch/descriptions/executable.py @@ -198,7 +198,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/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 f9c8ce896..03d2a938e 100644 --- a/launch/launch/frontend/parser.py +++ b/launch/launch/frontend/parser.py @@ -23,6 +23,7 @@ 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 @@ -138,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 @@ -149,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 @@ -161,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: @@ -181,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() @@ -193,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) @@ -202,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() @@ -211,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. @@ -223,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 @@ -256,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() From 6f44cb3ea2d6883f1b66fa93b9bf31898d668735 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 09:27:27 +0900 Subject: [PATCH 36/53] Ignore untyped imports --- launch/setup.cfg | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 launch/setup.cfg 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 From f6dda61783fae559b0a0cf1d6e7b2b45a231ea9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 09:57:48 +0900 Subject: [PATCH 37/53] More fixes to python expression But it's super weird --- .../launch/event_handlers/on_action_event_base.py | 2 +- launch/launch/substitutions/python_expression.py | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/launch/launch/event_handlers/on_action_event_base.py b/launch/launch/event_handlers/on_action_event_base.py index 9adcff9c4..6b3e096ca 100644 --- a/launch/launch/event_handlers/on_action_event_base.py +++ b/launch/launch/event_handlers/on_action_event_base.py @@ -111,7 +111,7 @@ def handle(self, event: Event, context: LaunchContext) -> Optional[SomeEntitiesT 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/substitutions/python_expression.py b/launch/launch/substitutions/python_expression.py index 98a75c45c..26a89a665 100644 --- a/launch/launch/substitutions/python_expression.py +++ b/launch/launch/substitutions/python_expression.py @@ -70,8 +70,18 @@ def parse(cls, data: Sequence[SomeSubstitutionsType]): # whose contents are comma-separated module names kwargs['python_modules'] = [] # Check if we got empty list from XML - if len(data[1]) > 0: - modules_str = data[1][0].perform(None) + # 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 list of either strings or substitutions, but this says that we're + # getting a list of things? + # 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[1][0].perform(None) # type: ignore kwargs['python_modules'] = [module.strip() for module in modules_str.split(',')] return cls, kwargs From 1e3cf1b58cc1e263233e360d43093fbe235d5727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 11:01:24 +0900 Subject: [PATCH 38/53] Fixup simple bug --- launch/launch/actions/execute_local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch/launch/actions/execute_local.py b/launch/launch/actions/execute_local.py index 91acd0fac..d0fa26a06 100644 --- a/launch/launch/actions/execute_local.py +++ b/launch/launch/actions/execute_local.py @@ -194,7 +194,7 @@ def __init__( 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(self.__output) + self.__output = normalize_to_list_of_substitutions(tmp_output) else: self.__output = tmp_output self.__output_format = output_format From 0abf6a7d54f46189efb1b708cbad365de77f19f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 11:01:32 +0900 Subject: [PATCH 39/53] Remove unused import --- launch/launch/actions/execute_process.py | 1 - 1 file changed, 1 deletion(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 03bdc3348..cfe24058b 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -20,7 +20,6 @@ from typing import List from typing import Optional from typing import Text -from typing import cast from .execute_local import ExecuteLocal From c4afced752fc6b56fd57c2e68a1bf60f544e4b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 11:39:29 +0900 Subject: [PATCH 40/53] Ignore docs --- launch/doc/source/conf.py | 1 + 1 file changed, 1 insertion(+) 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 From bddeb050b7f07d0aa3dcc75f143ef1cbf48db066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 11:52:18 +0900 Subject: [PATCH 41/53] Just a few typechecks --- launch/examples/disable_emulate_tty_counters.py | 5 ++--- launch/examples/launch_counters.py | 5 ++--- launch/test/launch/frontend/test_substitutions.py | 6 ++++-- launch/test/launch/utilities/test_signal_management.py | 3 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/launch/examples/disable_emulate_tty_counters.py b/launch/examples/disable_emulate_tty_counters.py index 231937ada..efd84e319 100755 --- a/launch/examples/disable_emulate_tty_counters.py +++ b/launch/examples/disable_emulate_tty_counters.py @@ -37,10 +37,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..0599ac260 100755 --- a/launch/examples/launch_counters.py +++ b/launch/examples/launch_counters.py @@ -55,10 +55,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/test/launch/frontend/test_substitutions.py b/launch/test/launch/frontend/test_substitutions.py index 715f3a0c6..5af88f4b6 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): @@ -270,7 +272,7 @@ def test_eval_subst_multiple_modules_alt_syntax(): 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/utilities/test_signal_management.py b/launch/test/launch/utilities/test_signal_management.py index 62dffbc3a..be90750dd 100644 --- a/launch/test/launch/utilities/test_signal_management.py +++ b/launch/test/launch/utilities/test_signal_management.py @@ -18,6 +18,7 @@ import functools import platform import signal +import sys from launch.utilities import AsyncSafeSignalManager @@ -51,7 +52,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 From d75b9ce80dd717a4dea93a80e91bb97e39a4164c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 12:33:51 +0900 Subject: [PATCH 42/53] Ignore YAML checking --- launch/launch/utilities/type_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/launch/launch/utilities/type_utils.py b/launch/launch/utilities/type_utils.py index 8941a2910..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 From d26320b8eda2f3c651cb05490a9476906dd54e9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 12:41:34 +0900 Subject: [PATCH 43/53] Check for errors in loading spec --- .../launch_description_sources/python_launch_file_utilities.py | 2 ++ 1 file changed, 2 insertions(+) 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..d777068a6 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 From 1ac22e37d09e9d55c775daa6e00e25bbf0b5be31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 12:41:50 +0900 Subject: [PATCH 44/53] Ignore a typing error in here --- launch/test/launch/utilities/test_type_utils.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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', From b17f51ba6db19204f4fe53d84242218da113fd13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 13:09:54 +0900 Subject: [PATCH 45/53] Ignore SIGBREAK --- launch/test/launch/utilities/test_signal_management.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/launch/test/launch/utilities/test_signal_management.py b/launch/test/launch/utilities/test_signal_management.py index be90750dd..16b2fa264 100644 --- a/launch/test/launch/utilities/test_signal_management.py +++ b/launch/test/launch/utilities/test_signal_management.py @@ -47,7 +47,10 @@ def _wrapper(*args, **kwargs): if platform.system() == 'Windows': # NOTE(hidmic): this is risky, but we have few options. SIGNAL = signal.SIGINT - ANOTHER_SIGNAL = signal.SIGBREAK + # We need to ignore this as mypy does not recognize `platform.system` + # The other option would be to switch this check to `sys.platform` + # which is correctly recognized by mypy. + ANOTHER_SIGNAL = signal.SIGBREAK # type: ignore else: SIGNAL = signal.SIGUSR1 ANOTHER_SIGNAL = signal.SIGUSR2 From 07f50e20a818e798060011cdc624eae4a4391b9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 13:21:18 +0900 Subject: [PATCH 46/53] Fixup pep257 --- launch/launch/utilities/normalize_to_list_of_entities_impl.py | 1 - 1 file changed, 1 deletion(-) diff --git a/launch/launch/utilities/normalize_to_list_of_entities_impl.py b/launch/launch/utilities/normalize_to_list_of_entities_impl.py index cb4db7b87..8b5d6a935 100644 --- a/launch/launch/utilities/normalize_to_list_of_entities_impl.py +++ b/launch/launch/utilities/normalize_to_list_of_entities_impl.py @@ -23,7 +23,6 @@ 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): From fd6efed9e53d9fa9559837f7c0f1433d91d0cce6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 13:37:03 +0900 Subject: [PATCH 47/53] Fixup flake8 errors --- .../examples/disable_emulate_tty_counters.py | 1 - launch/examples/launch_counters.py | 1 - launch/launch/actions/execute_local.py | 17 ++++++++++--- .../launch/actions/register_event_handler.py | 3 ++- launch/launch/descriptions/executable.py | 1 - .../launch/event_handlers/on_process_exit.py | 10 ++++++-- launch/launch/event_handlers/on_shutdown.py | 4 ++- launch/launch/frontend/entity.py | 25 ++++++++++--------- launch/launch/frontend/parser.py | 10 ++++---- launch/launch/launch_introspector.py | 4 ++- launch/launch/logging/handlers.py | 1 + .../normalize_to_list_of_entities_impl.py | 3 ++- .../launch/frontend/test_substitutions.py | 5 +++- 13 files changed, 54 insertions(+), 31 deletions(-) diff --git a/launch/examples/disable_emulate_tty_counters.py b/launch/examples/disable_emulate_tty_counters.py index efd84e319..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 diff --git a/launch/examples/launch_counters.py b/launch/examples/launch_counters.py index 0599ac260..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 diff --git a/launch/launch/actions/execute_local.py b/launch/launch/actions/execute_local.py index d0fa26a06..1fa630c8a 100644 --- a/launch/launch/actions/execute_local.py +++ b/launch/launch/actions/execute_local.py @@ -191,7 +191,9 @@ def __init__( self.__emulate_tty = emulate_tty # 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) + 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) @@ -589,9 +591,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 self.__shutdown_future is not None 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`) @@ -692,7 +699,9 @@ def execute(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEnti self.__logger = launch.logging.get_logger(name) if not isinstance(self.__output, dict): self.__stdout_logger, self.__stderr_logger = \ - launch.logging.get_output_loggers(name, perform_substitutions(context, self.__output)) + 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) diff --git a/launch/launch/actions/register_event_handler.py b/launch/launch/actions/register_event_handler.py index 30913f785..54f8c15f2 100644 --- a/launch/launch/actions/register_event_handler.py +++ b/launch/launch/actions/register_event_handler.py @@ -60,5 +60,6 @@ def describe_conditional_sub_entities(self) -> List[Tuple[ event_handler_description = self.__event_handler.describe() return [ - (event_handler_description[0], normalize_to_list_of_entities(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/descriptions/executable.py b/launch/launch/descriptions/executable.py index 1c121f232..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 diff --git a/launch/launch/event_handlers/on_process_exit.py b/launch/launch/event_handlers/on_process_exit.py index a121be50c..41efb2bf9 100644 --- a/launch/launch/event_handlers/on_process_exit.py +++ b/launch/launch/event_handlers/on_process_exit.py @@ -55,8 +55,14 @@ def __init__( """Create an OnProcessExit event handler.""" from ..actions import ExecuteLocal # noqa: F811 super().__init__( - action_matcher=cast(Optional[Union[Callable[['Action'], bool], 'Action']], target_action), - on_event=cast(Union[SomeEntitiesType, Callable[[Event, LaunchContext], Optional[SomeEntitiesType]]], 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_shutdown.py b/launch/launch/event_handlers/on_shutdown.py index 76ef4d1d2..c5ad1928c 100644 --- a/launch/launch/event_handlers/on_shutdown.py +++ b/launch/launch/event_handlers/on_shutdown.py @@ -31,7 +31,9 @@ from ..launch_context import LaunchContext # noqa: F401 -def gen_handler(entities: SomeEntitiesType) -> Callable[[Shutdown, 'LaunchContext'], SomeEntitiesType]: +def gen_handler( + entities: SomeEntitiesType + ) -> Callable[[Shutdown, 'LaunchContext'], SomeEntitiesType]: def handler(event: Shutdown, context: 'LaunchContext') -> SomeEntitiesType: return entities return handler diff --git a/launch/launch/frontend/entity.py b/launch/launch/frontend/entity.py index cbf468c4a..fd741b399 100644 --- a/launch/launch/frontend/entity.py +++ b/launch/launch/frontend/entity.py @@ -19,16 +19,13 @@ from typing import Optional from typing import Text from typing import Type -from typing import Union from typing import TypeVar from typing import overload -from launch.utilities.type_utils import AllowedTypesType -from launch.utilities.type_utils import AllowedValueType - TargetType = TypeVar("TargetType") + class Entity: """Single item in the intermediate front_end representation.""" @@ -55,31 +52,35 @@ def children(self) -> List['Entity']: # 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: + 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], optional: Literal[True], can_be_str: bool = True) -> Optional[TargetType]: + 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], can_be_str: bool = True) -> TargetType: + 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], can_be_str: bool = True) -> str: + 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], can_be_str: bool = True) -> Optional[str]: + 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: + def get_attr(self, name: Text, *, can_be_str: bool = True) -> str: # noqa: F811 ... - - def get_attr( + def get_attr( # noqa: F811 self, name, *, diff --git a/launch/launch/frontend/parser.py b/launch/launch/frontend/parser.py index 03d2a938e..6576db966 100644 --- a/launch/launch/frontend/parser.py +++ b/launch/launch/frontend/parser.py @@ -30,11 +30,6 @@ import warnings import sys -if sys.version_info >= (3, 8): - import importlib.metadata as importlib_metadata -else: - import importlib_metadata - from .entity import Entity from .expose import instantiate_action from .parse_substitution import parse_if_substitutions @@ -47,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 diff --git a/launch/launch/launch_introspector.py b/launch/launch/launch_introspector.py index 78694ed93..c53a43b42 100644 --- a/launch/launch/launch_introspector.py +++ b/launch/launch/launch_introspector.py @@ -92,7 +92,9 @@ def format_event_handler(event_handler: BaseEventHandler) -> List[Text]: 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)))) + result.extend( + indent(format_entities(cast(Iterable[LaunchDescriptionEntity], entities))) + ) return result else: return ["EventHandler('{}')".format(hex(id(event_handler)))] diff --git a/launch/launch/logging/handlers.py b/launch/launch/logging/handlers.py index b93f4c710..a172c53c0 100644 --- a/launch/launch/logging/handlers.py +++ b/launch/launch/logging/handlers.py @@ -17,6 +17,7 @@ import sys import types + def with_per_logger_formatting(cls): """Add per logger formatting capabilities to the given logging.Handler.""" class _trait(cls): diff --git a/launch/launch/utilities/normalize_to_list_of_entities_impl.py b/launch/launch/utilities/normalize_to_list_of_entities_impl.py index 8b5d6a935..58d1e1355 100644 --- a/launch/launch/utilities/normalize_to_list_of_entities_impl.py +++ b/launch/launch/utilities/normalize_to_list_of_entities_impl.py @@ -21,7 +21,8 @@ from ..launch_description_entity import LaunchDescriptionEntity -def normalize_to_list_of_entities(entities: Iterable[SomeEntitiesType]) -> List[LaunchDescriptionEntity]: +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: diff --git a/launch/test/launch/frontend/test_substitutions.py b/launch/test/launch/frontend/test_substitutions.py index 5af88f4b6..1a5ce9538 100644 --- a/launch/test/launch/frontend/test_substitutions.py +++ b/launch/test/launch/frontend/test_substitutions.py @@ -272,7 +272,10 @@ def test_eval_subst_multiple_modules_alt_syntax(): def expand_cmd_subs(cmd_subs: List[SomeSubstitutionsType]): - return [perform_substitutions_without_context(normalize_to_list_of_substitutions(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(): From 72feb5b700cf467619b54c2e4ea4948b5041085f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 13:40:11 +0900 Subject: [PATCH 48/53] Another small bug --- launch/launch/substitutions/python_expression.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/launch/launch/substitutions/python_expression.py b/launch/launch/substitutions/python_expression.py index 26a89a665..959c50510 100644 --- a/launch/launch/substitutions/python_expression.py +++ b/launch/launch/substitutions/python_expression.py @@ -77,11 +77,11 @@ def parse(cls, data: Sequence[SomeSubstitutionsType]): modules = list(data[1]) if len(modules) > 0: # XXX: What is going on here: the type annotation says we should get - # a list of either strings or substitutions, but this says that we're - # getting a list of things? + # 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[1][0].perform(None) # type: ignore + modules_str = modules[0].perform(None) # type: ignore kwargs['python_modules'] = [module.strip() for module in modules_str.split(',')] return cls, kwargs From 8850288f8d5b9181c9d5fd2c8082b8655c3a32f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Fri, 17 Feb 2023 13:48:32 +0900 Subject: [PATCH 49/53] Fixup flake8 in testing --- launch_testing/launch_testing/actions/test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/launch_testing/launch_testing/actions/test.py b/launch_testing/launch_testing/actions/test.py index 2e72eea08..bebc537b6 100644 --- a/launch_testing/launch_testing/actions/test.py +++ b/launch_testing/launch_testing/actions/test.py @@ -56,7 +56,9 @@ def timeout(self): """Getter for timeout.""" return self.__timeout - def __on_process_exit(self, event: Event, context: LaunchContext) -> Optional[SomeEntitiesType]: + def __on_process_exit( + self, event: Event, context: LaunchContext + ) -> Optional[SomeEntitiesType]: """On shutdown event.""" if self.__timer: self.__timer.cancel() From 8e9a8f549ddf4e760d42d14982c16397dc44fcac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Mon, 27 Feb 2023 16:27:43 +0900 Subject: [PATCH 50/53] Reintroduce SomeActionsType It is aliased to SomeEntitiesType, and comes with a deprecation warning! --- launch/launch/some_actions_type.py | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 launch/launch/some_actions_type.py diff --git a/launch/launch/some_actions_type.py b/launch/launch/some_actions_type.py new file mode 100644 index 000000000..02187ef53 --- /dev/null +++ b/launch/launch/some_actions_type.py @@ -0,0 +1,33 @@ +# 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 SomeActionsType type. + +.. deprecated:: 1.4.2 + Replaced by the more aptly named 'SomeEntitiesType' +""" + +import warnings + +from .some_entities_type import SomeEntitiesType, SomeEntitiesType_types_tuple + +warnings.warn( + "The 'SomeActionsType' type is deprecated. Use 'SomeEntitiesType' in your type" + " annotations instead!", + UserWarning, +) + +SomeActionsType = SomeEntitiesType +SomeActionsType_types_tuple = SomeEntitiesType_types_tuple From 32fa4ccd437bba8d4163aa5ecc12c7555f05cfb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Tue, 28 Feb 2023 13:20:47 +0900 Subject: [PATCH 51/53] Make flake8 happy! - Fixup some quotes and line lengths - Many orders needed to be reordered --- launch/launch/action.py | 2 +- launch/launch/actions/opaque_coroutine.py | 4 ++-- launch/launch/conditions/launch_configuration_equals.py | 2 +- launch/launch/frontend/entity.py | 2 +- launch/launch/frontend/parser.py | 2 +- launch/launch/launch_introspector.py | 2 +- launch/launch/logging/__init__.py | 2 +- launch/launch/some_actions_type.py | 2 +- launch/launch/substitutions/anon_name.py | 2 +- launch/launch/substitutions/command.py | 2 +- launch/launch/substitutions/environment_variable.py | 2 +- launch/launch/substitutions/equals_substitution.py | 4 ++-- launch/launch/substitutions/find_executable.py | 2 +- launch/launch/substitutions/launch_configuration.py | 2 +- launch/launch/utilities/__init__.py | 2 +- launch/launch/utilities/normalize_to_list_of_entities_impl.py | 2 +- launch/test/test_mypy.py | 4 ++-- 17 files changed, 20 insertions(+), 20 deletions(-) diff --git a/launch/launch/action.py b/launch/launch/action.py index ead786ae9..4834680f9 100644 --- a/launch/launch/action.py +++ b/launch/launch/action.py @@ -14,12 +14,12 @@ """Module for Action class.""" +from typing import Dict from typing import Iterable from typing import List from typing import Optional from typing import Text from typing import Tuple -from typing import Dict from .condition import Condition from .launch_context import LaunchContext diff --git a/launch/launch/actions/opaque_coroutine.py b/launch/launch/actions/opaque_coroutine.py index 98491bac8..3628075ef 100644 --- a/launch/launch/actions/opaque_coroutine.py +++ b/launch/launch/actions/opaque_coroutine.py @@ -17,13 +17,13 @@ import asyncio import collections.abc from typing import Any +from typing import Awaitable +from typing import Callable from typing import Dict from typing import Iterable from typing import List from typing import Optional from typing import Text -from typing import Callable -from typing import Awaitable from ..action import Action from ..event import Event diff --git a/launch/launch/conditions/launch_configuration_equals.py b/launch/launch/conditions/launch_configuration_equals.py index 8439df7a2..59c059ec2 100644 --- a/launch/launch/conditions/launch_configuration_equals.py +++ b/launch/launch/conditions/launch_configuration_equals.py @@ -14,9 +14,9 @@ """Module for LaunchConfigurationEquals class.""" +from typing import List from typing import Optional from typing import Text -from typing import List import warnings from ..condition import Condition diff --git a/launch/launch/frontend/entity.py b/launch/launch/frontend/entity.py index fd741b399..a6dee595b 100644 --- a/launch/launch/frontend/entity.py +++ b/launch/launch/frontend/entity.py @@ -17,10 +17,10 @@ from typing import List from typing import Literal from typing import Optional +from typing import overload from typing import Text from typing import Type from typing import TypeVar -from typing import overload TargetType = TypeVar("TargetType") diff --git a/launch/launch/frontend/parser.py b/launch/launch/frontend/parser.py index 6576db966..50e6e860e 100644 --- a/launch/launch/frontend/parser.py +++ b/launch/launch/frontend/parser.py @@ -17,6 +17,7 @@ import itertools import os.path +import sys import traceback from typing import List from typing import Optional @@ -28,7 +29,6 @@ from typing import TYPE_CHECKING from typing import Union import warnings -import sys from .entity import Entity from .expose import instantiate_action diff --git a/launch/launch/launch_introspector.py b/launch/launch/launch_introspector.py index c53a43b42..1abcbcef4 100644 --- a/launch/launch/launch_introspector.py +++ b/launch/launch/launch_introspector.py @@ -15,8 +15,8 @@ """Module for the LaunchIntrospector class.""" from typing import cast -from typing import List from typing import Iterable +from typing import List from typing import Text from .action import Action diff --git a/launch/launch/logging/__init__.py b/launch/launch/logging/__init__.py index 35c172656..9c79efc9c 100644 --- a/launch/launch/logging/__init__.py +++ b/launch/launch/logging/__init__.py @@ -24,8 +24,8 @@ import socket import sys -from typing import List from typing import Any +from typing import List from . import handlers diff --git a/launch/launch/some_actions_type.py b/launch/launch/some_actions_type.py index 02187ef53..c5c4030c8 100644 --- a/launch/launch/some_actions_type.py +++ b/launch/launch/some_actions_type.py @@ -25,7 +25,7 @@ warnings.warn( "The 'SomeActionsType' type is deprecated. Use 'SomeEntitiesType' in your type" - " annotations instead!", + ' annotations instead!', UserWarning, ) diff --git a/launch/launch/substitutions/anon_name.py b/launch/launch/substitutions/anon_name.py index 6bb79e1f9..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 Sequence from typing import List +from typing import Sequence from typing import Text from ..frontend import expose_substitution diff --git a/launch/launch/substitutions/command.py b/launch/launch/substitutions/command.py index 86d71e019..0aed7a9d6 100644 --- a/launch/launch/substitutions/command.py +++ b/launch/launch/substitutions/command.py @@ -17,8 +17,8 @@ import os import shlex import subprocess -from typing import Sequence from typing import List +from typing import Sequence from typing import Text from typing import Union diff --git a/launch/launch/substitutions/environment_variable.py b/launch/launch/substitutions/environment_variable.py index 7f13bf94e..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 Sequence from typing import List from typing import Optional +from typing import Sequence from typing import Text from .substitution_failure import SubstitutionFailure diff --git a/launch/launch/substitutions/equals_substitution.py b/launch/launch/substitutions/equals_substitution.py index ca5cf50f0..02c91d071 100644 --- a/launch/launch/substitutions/equals_substitution.py +++ b/launch/launch/substitutions/equals_substitution.py @@ -17,13 +17,13 @@ import math from typing import Any -from typing import Sequence +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 -from typing import cast from ..frontend import expose_substitution from ..launch_context import LaunchContext diff --git a/launch/launch/substitutions/find_executable.py b/launch/launch/substitutions/find_executable.py index 3177583a3..1124428ac 100644 --- a/launch/launch/substitutions/find_executable.py +++ b/launch/launch/substitutions/find_executable.py @@ -14,8 +14,8 @@ """Module for the FindExecutable substitution.""" -from typing import Sequence from typing import List +from typing import Sequence from typing import Text from osrf_pycommon.process_utils import which # type: ignore diff --git a/launch/launch/substitutions/launch_configuration.py b/launch/launch/substitutions/launch_configuration.py index 483bbd475..4698129f9 100644 --- a/launch/launch/substitutions/launch_configuration.py +++ b/launch/launch/substitutions/launch_configuration.py @@ -19,9 +19,9 @@ from typing import Iterable from typing import List from typing import Optional +from typing import Sequence from typing import Text from typing import Union -from typing import Sequence from .substitution_failure import SubstitutionFailure from ..frontend import expose_substitution diff --git a/launch/launch/utilities/__init__.py b/launch/launch/utilities/__init__.py index bd0908ab2..ca618adee 100644 --- a/launch/launch/utilities/__init__.py +++ b/launch/launch/utilities/__init__.py @@ -17,8 +17,8 @@ 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_substitutions_impl import normalize_to_list_of_substitutions 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 from .visit_all_entities_and_collect_futures_impl import 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 index 58d1e1355..a41652d41 100644 --- a/launch/launch/utilities/normalize_to_list_of_entities_impl.py +++ b/launch/launch/utilities/normalize_to_list_of_entities_impl.py @@ -17,8 +17,8 @@ from typing import Iterable from typing import List -from ..some_entities_type import SomeEntitiesType from ..launch_description_entity import LaunchDescriptionEntity +from ..some_entities_type import SomeEntitiesType def normalize_to_list_of_entities(entities: Iterable[SomeEntitiesType]) ->\ diff --git a/launch/test/test_mypy.py b/launch/test/test_mypy.py index 056094604..31db1869b 100644 --- a/launch/test/test_mypy.py +++ b/launch/test/test_mypy.py @@ -12,12 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import pytest 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!" + assert rc == 0, 'Found type errors!' From 4b170970ee2c739906971cb87379db8aa9bb6b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Wed, 1 Mar 2023 09:18:48 +0900 Subject: [PATCH 52/53] One more round of lint... --- launch/launch/actions/include_launch_description.py | 2 +- launch/launch/frontend/entity.py | 2 +- .../launch_description_sources/python_launch_file_utilities.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/launch/launch/actions/include_launch_description.py b/launch/launch/actions/include_launch_description.py index 2d3395d52..d022d5ee5 100644 --- a/launch/launch/actions/include_launch_description.py +++ b/launch/launch/actions/include_launch_description.py @@ -78,7 +78,7 @@ def __init__( if not isinstance(launch_description_source, LaunchDescriptionSource): launch_description_source = AnyLaunchDescriptionSource(launch_description_source) self.__launch_description_source = launch_description_source - self.__launch_arguments = tuple() if launch_arguments is None else tuple(launch_arguments) + self.__launch_arguments = () if launch_arguments is None else tuple(launch_arguments) self.__logger = launch.logging.get_logger(__name__) @classmethod diff --git a/launch/launch/frontend/entity.py b/launch/launch/frontend/entity.py index a6dee595b..3816e6115 100644 --- a/launch/launch/frontend/entity.py +++ b/launch/launch/frontend/entity.py @@ -23,7 +23,7 @@ from typing import TypeVar -TargetType = TypeVar("TargetType") +TargetType = TypeVar('TargetType') class 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 d777068a6..87011c7f9 100644 --- a/launch/launch/launch_description_sources/python_launch_file_utilities.py +++ b/launch/launch/launch_description_sources/python_launch_file_utilities.py @@ -34,7 +34,7 @@ def load_python_launch_file_as_module(python_launch_file_path: Text) -> ModuleTy 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!") + raise RuntimeError('Failed to load module spec!') mod = module_from_spec(spec) loader.exec_module(mod) return mod From a2e87bb76e8e6dca409eb91098e450736a0cab9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Audren?= Date: Thu, 2 Mar 2023 09:42:58 +0900 Subject: [PATCH 53/53] Use sys.platform to check for Windows This check is recognized by mypy properly! --- launch/test/launch/utilities/test_signal_management.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/launch/test/launch/utilities/test_signal_management.py b/launch/test/launch/utilities/test_signal_management.py index 16b2fa264..d95d2a9c7 100644 --- a/launch/test/launch/utilities/test_signal_management.py +++ b/launch/test/launch/utilities/test_signal_management.py @@ -16,7 +16,6 @@ import asyncio import functools -import platform import signal import sys @@ -44,13 +43,10 @@ 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 - # We need to ignore this as mypy does not recognize `platform.system` - # The other option would be to switch this check to `sys.platform` - # which is correctly recognized by mypy. - ANOTHER_SIGNAL = signal.SIGBREAK # type: ignore + ANOTHER_SIGNAL = signal.SIGBREAK else: SIGNAL = signal.SIGUSR1 ANOTHER_SIGNAL = signal.SIGUSR2