Allow Path in substitutions, instead of requiring cast to str (#873)

Signed-off-by: Emerson Knapp <[email protected]>
(cherry picked from commit 49bcf2d)

# Conflicts:
#	launch/launch/substitutions/python_expression.py
#	launch_xml/test/launch_xml/test_executable.py

Author: emersonknapp

launch/launch/launch_description_sources/python_launch_file_utilities.py

@@ -17,8 +17,10 @@
 from importlib.machinery import SourceFileLoader
 from importlib.util import module_from_spec
 from importlib.util import spec_from_loader
+from pathlib import Path
 from types import ModuleType
 from typing import Text
+from typing import Union
 
 from ..launch_description import LaunchDescription
 
@@ -29,17 +31,17 @@ class InvalidPythonLaunchFileError(Exception):
     ...
 
 
-def load_python_launch_file_as_module(python_launch_file_path: Text) -> ModuleType:
+def load_python_launch_file_as_module(python_launch_file_path: Union[Text, Path]) -> ModuleType:
     """Load a given Python launch file (by path) as a Python module."""
-    loader = SourceFileLoader('python_launch_file', python_launch_file_path)
+    loader = SourceFileLoader('python_launch_file', str(python_launch_file_path))
     spec = spec_from_loader(loader.name, loader)
     mod = module_from_spec(spec)
     loader.exec_module(mod)
     return mod
 
 
 def get_launch_description_from_python_launch_file(
-    python_launch_file_path: Text
+    python_launch_file_path: Union[Text, Path]
 ) -> LaunchDescription:
     """
     Load a given Python launch file (by path), and return the launch description from it.

launch/launch/some_substitutions_type.py

@@ -15,6 +15,7 @@
 """Module for SomeSubstitutionsType type."""
 
 import collections.abc
+from pathlib import Path
 from typing import Iterable
 from typing import Text
 from typing import Union
@@ -23,12 +24,14 @@
 
 SomeSubstitutionsType = Union[
     Text,
+    Path,
     Substitution,
-    Iterable[Union[Text, Substitution]],
+    Iterable[Union[Text, Path, Substitution]],
 ]
 
 SomeSubstitutionsType_types_tuple = (
     str,
+    Path,
     Substitution,
     collections.abc.Iterable,
 )

launch/launch/substitutions/path_join_substitution.py

@@ -14,7 +14,7 @@
 
 """Module for the PathJoinSubstitution substitution."""
 
-import os
+from pathlib import Path
 from typing import Iterable
 from typing import List
 from typing import Text
@@ -86,7 +86,7 @@ def perform(self, context: LaunchContext) -> Text:
             perform_substitutions(context, component_substitutions)
             for component_substitutions in self.substitutions
         ]
-        return os.path.join(*path_components)
+        return str(Path(*path_components))
 
     def __truediv__(self, additional_path: SomeSubstitutionsType) -> 'PathJoinSubstitution':
         """Join path substitutions using the / operator, mimicking pathlib.Path operation."""

launch/launch/substitutions/python_expression.py

@@ -15,8 +15,13 @@
 """Module for the PythonExpression substitution."""
 
 import collections.abc
+<<<<<<< HEAD
 import math
 from typing import Iterable
+=======
+import importlib
+from pathlib import Path
+>>>>>>> 49bcf2d (Allow Path in substitutions, instead of requiring cast to str (#873))
 from typing import List
 from typing import Text
 
@@ -53,9 +58,36 @@ def __init__(self, expression: SomeSubstitutionsType) -> None:
     @classmethod
     def parse(cls, data: Iterable[SomeSubstitutionsType]):
         """Parse `PythonExpression` substitution."""
+<<<<<<< HEAD
         if len(data) != 1:
             raise TypeError('eval substitution expects 1 argument')
         return cls, {'expression': data[0]}
+=======
+        if len(data) < 1 or len(data) > 2:
+            raise TypeError('eval substitution expects 1 or 2 arguments')
+        kwargs = {}
+        kwargs['expression'] = data[0]
+        if len(data) == 2:
+            # We get a text substitution from XML,
+            # whose contents are comma-separated module names
+            kwargs['python_modules'] = []
+            # Check if we got empty list from XML
+            # Ensure that we got a list!
+            assert not isinstance(data[1], str)
+            assert not isinstance(data[1], Path)
+            assert not isinstance(data[1], Substitution)
+            # Modules
+            modules = list(data[1])
+            if len(modules) > 0:
+                # XXX: What is going on here: the type annotation says we should get
+                # a either strings or substitutions, but this says that we're
+                # getting a substitution always?
+                # Moreover, `perform` is called with `None`, which is not acceptable
+                # for any substitution as far as I know (should be an empty launch context?)
+                modules_str = modules[0].perform(None)  # type: ignore
+                kwargs['python_modules'] = [module.strip() for module in modules_str.split(',')]
+        return cls, kwargs
+>>>>>>> 49bcf2d (Allow Path in substitutions, instead of requiring cast to str (#873))
 
     @property
     def expression(self) -> List[Substitution]:

launch/launch/utilities/normalize_to_list_of_substitutions_impl.py

@@ -14,6 +14,7 @@
 
 """Module for the normalize_to_list_of_substitutions() utility function."""
 
+from pathlib import Path
 from typing import cast
 from typing import Iterable
 from typing import List
@@ -31,14 +32,14 @@ def normalize_to_list_of_substitutions(subs: SomeSubstitutionsType) -> List[Subs
     def normalize(x):
         if isinstance(x, Substitution):
             return x
-        if isinstance(x, str):
-            return TextSubstitution(text=x)
+        if isinstance(x, (str, Path)):
+            return TextSubstitution(text=str(x))
         raise TypeError(
             "Failed to normalize given item of type '{}', when only "
             "'str' or 'launch.Substitution' were expected.".format(type(x)))
 
-    if isinstance(subs, str):
-        return [TextSubstitution(text=subs)]
+    if isinstance(subs, (str, Path)):
+        return [TextSubstitution(text=str(subs))]
     if is_a_subclass(subs, Substitution):
         return [cast(Substitution, subs)]
     return [normalize(y) for y in cast(Iterable, subs)]

launch/launch/utilities/typing_file_path.py

@@ -13,7 +13,8 @@
 # limitations under the License.
 
 import os
+from pathlib import Path
 import typing
 
 # Type of a filesystem path
-FilePath = typing.TypeVar('FilePath', str, bytes, os.PathLike)
+FilePath = typing.TypeVar('FilePath', str, bytes, os.PathLike, Path)

launch/test/launch/actions/test_include_launch_description.py

@@ -14,7 +14,7 @@
 
 """Tests for the IncludeLaunchDescription action class."""
 
-import os
+from pathlib import Path
 
 from launch import LaunchContext
 from launch import LaunchDescription
@@ -74,7 +74,7 @@ def test_include_launch_description_launch_file_location():
     assert lc1.locals.current_launch_file_directory == '<script>'
     assert action.get_asyncio_future() is None
 
-    this_file = os.path.abspath(__file__)
+    this_file = Path(__file__).absolute()
     ld2 = LaunchDescription()
     action2 = IncludeLaunchDescription(LaunchDescriptionSource(ld2, this_file))
     assert 'IncludeLaunchDescription' in action2.describe()
@@ -83,7 +83,7 @@ def test_include_launch_description_launch_file_location():
     lc2 = LaunchContext()
     # Result should only contain the launch description as there are no launch arguments.
     assert action2.visit(lc2) == [ld2]
-    assert lc2.locals.current_launch_file_directory == os.path.dirname(this_file)
+    assert lc2.locals.current_launch_file_directory == str(this_file.parent)
     assert action2.get_asyncio_future() is None
 
 
@@ -209,11 +209,8 @@ def test_include_launch_description_launch_arguments():
 
 def test_include_python():
     """Test including Python, with and without explicit PythonLaunchDescriptionSource."""
-    this_dir = os.path.dirname(os.path.abspath(__file__))
-    simple_launch_file_path = os.path.join(this_dir,
-                                           '..',
-                                           'launch_description_source',
-                                           'simple.launch.py')
+    this_dir = Path(__file__).parent
+    simple_launch_file_path = this_dir.parent / 'launch_description_source' / 'simple.launch.py'
 
     # Explicitly construct with PythonLaunchDescriptionSource
     plds = PythonLaunchDescriptionSource(simple_launch_file_path)
@@ -232,4 +229,4 @@ def test_include_python():
         assert action.get_asyncio_future() is None
         assert len(action.launch_arguments) == 0
 
-        assert action.launch_description_source.location == simple_launch_file_path
+        assert action.launch_description_source.location == str(simple_launch_file_path)

launch/test/launch/launch_description_source/test_python_launch_description_source.py

@@ -14,7 +14,7 @@
 
 """Tests for the PythonLaunchDescriptionSource class."""
 
-import os
+from pathlib import Path
 
 from launch import LaunchContext
 from launch.launch_description_sources import InvalidPythonLaunchFileError
@@ -25,17 +25,17 @@
 
 def test_python_launch_description_source():
     """Test the PythonLaunchDescriptionSource class."""
-    this_dir = os.path.dirname(os.path.abspath(__file__))
-    simple_launch_file_path = os.path.join(this_dir, 'simple.launch.py')
+    this_dir = Path(__file__).parent
+    simple_launch_file_path = this_dir / 'simple.launch.py'
     plds = PythonLaunchDescriptionSource(simple_launch_file_path)
     assert 'python launch file' in plds.method
     assert 'launch.substitutions.text_substitution.TextSubstitution' in plds.location
     ld = plds.get_launch_description(LaunchContext())
-    assert plds.location == simple_launch_file_path
+    assert plds.location == str(simple_launch_file_path)
     assert 0 == len(ld.entities)
 
     with pytest.raises(InvalidPythonLaunchFileError):
-        plds = PythonLaunchDescriptionSource(os.path.join(this_dir, 'loadable_python_module.py'))
+        plds = PythonLaunchDescriptionSource(this_dir / 'loadable_python_module.py')
         ld = plds.get_launch_description(LaunchContext())
 
     with pytest.raises(FileNotFoundError):

launch/test/launch/launch_description_source/test_python_launch_file_utilities.py

@@ -14,7 +14,7 @@
 
 """Tests for the Python launch file utility functions."""
 
-import os
+from pathlib import Path
 import sys
 
 from launch.launch_description_sources import get_launch_description_from_python_launch_file
@@ -26,8 +26,8 @@
 
 def test_load_python_launch_file_as_module():
     """Test load_python_launch_file_as_module()."""
-    this_dir = os.path.dirname(os.path.abspath(__file__))
-    module = load_python_launch_file_as_module(os.path.join(this_dir, 'loadable_python_module.py'))
+    this_dir = Path(__file__).parent
+    module = load_python_launch_file_as_module(this_dir / 'loadable_python_module.py')
     assert sys.executable == module.some_function()
 
     with pytest.raises(FileNotFoundError):
@@ -36,13 +36,12 @@ def test_load_python_launch_file_as_module():
 
 def test_get_launch_description_from_python_launch_file():
     """Test get_launch_description_from_python_launch_file()."""
-    this_dir = os.path.dirname(os.path.abspath(__file__))
-    ld = get_launch_description_from_python_launch_file(os.path.join(this_dir, 'simple.launch.py'))
+    this_dir = Path(__file__).parent
+    ld = get_launch_description_from_python_launch_file(this_dir / 'simple.launch.py')
     assert 0 == len(ld.entities)
 
     with pytest.raises(InvalidPythonLaunchFileError):
-        ld = get_launch_description_from_python_launch_file(
-            os.path.join(this_dir, 'loadable_python_module.py'))
+        ld = get_launch_description_from_python_launch_file(this_dir / 'loadable_python_module.py')
 
     with pytest.raises(FileNotFoundError):
         ld = get_launch_description_from_python_launch_file('does_not_exist')

launch/test/launch/substitutions/test_command.py

@@ -29,14 +29,14 @@ def commands():
     this_dir = pathlib.Path(__file__).parent
 
     commands = {
-        'normal': str(this_dir / 'test_command' / 'normal_command.bash'),
-        'failing': str(this_dir / 'test_command' / 'failing_command.bash'),
-        'with_stderr': str(this_dir / 'test_command' / 'command_with_stderr.bash')
+        'normal': this_dir / 'test_command' / 'normal_command.bash',
+        'failing': this_dir / 'test_command' / 'failing_command.bash',
+        'with_stderr': this_dir / 'test_command' / 'command_with_stderr.bash'
     }
 
     if os.name == 'nt':
         for key, value in commands.items():
-            commands[key] = value.replace('bash', 'bat')
+            commands[key] = value.with_suffix('.bat')
     return commands