Pass modules to PythonExpression (#655)

* Added ability to pass Python modules to the PythonExpression substitution.
Allows eval of more expressions.
Signed-off-by: Blake Anderson

Signed-off-by: Blake Anderson <[email protected]>

* Fix style error WRT default arg whitespace.

Signed-off-by: Blake Anderson <[email protected]>

* Improve module passing to PythonExpression
It is now easier to pass multiple modules.
Also simpler syntax.

Signed-off-by: Blake Anderson <[email protected]>

* Tests for PythonExpression class

Signed-off-by: Blake Anderson <[email protected]>

* Fix style error for disallowed blank line.

Signed-off-by: Blake Anderson <[email protected]>

* Fixed wrong test method name.
Looks like a copy/paste error from another test.

Signed-off-by: Blake Anderson <[email protected]>

* Additional documentation for PythonExpression.

Signed-off-by: Blake Anderson <[email protected]>

* PythonExpression: update describe() method
Add more tests for describe()

Signed-off-by: Blake Anderson <[email protected]>

* PythonExpression: Take module names as substitutions

Signed-off-by: Blake Anderson <[email protected]>

* PythonExpression: Expression syntax change
Definitions from modules must be prepended by the module name.
Ex: 'sys.getrefcount' instead of just 'getrefcount'.
The math module is an exception for backwards compatibility.

Signed-off-by: Blake Anderson <[email protected]>

* PythonExpression: Rename modules to python_modules

Signed-off-by: Blake Anderson <[email protected]>

* PythonExpression: More robust wrt XML spacing syntax

Signed-off-by: Blake Anderson <[email protected]>

* PythonExpression: Flake8 appeasement.

Signed-off-by: Blake Anderson <[email protected]>

Signed-off-by: Blake Anderson <[email protected]>

launch/doc/source/architecture.rst

@@ -125,6 +125,7 @@ There are many possible variations of a substitution, but here are some of the c
 - :class:`launch.substitutions.PythonExpression`
 
   - This substitution will evaluate a python expression and get the result as a string.
+  - You may pass a list of Python modules to the constructor to allow the use of those modules in the evaluated expression.
 
 - :class:`launch.substitutions.LaunchConfiguration`
 

launch/launch/substitutions/python_expression.py

@@ -15,7 +15,7 @@
 """Module for the PythonExpression substitution."""
 
 import collections.abc
-import math
+import importlib
 from typing import Iterable
 from typing import List
 from typing import Text
@@ -37,7 +37,8 @@ class PythonExpression(Substitution):
     It also may contain math symbols and functions.
     """
 
-    def __init__(self, expression: SomeSubstitutionsType) -> None:
+    def __init__(self, expression: SomeSubstitutionsType,
+                 python_modules: SomeSubstitutionsType = ['math']) -> None:
         """Create a PythonExpression substitution."""
         super().__init__()
 
@@ -47,26 +48,61 @@ def __init__(self, expression: SomeSubstitutionsType) -> None:
             'expression',
             'PythonExpression')
 
+        ensure_argument_type(
+            python_modules,
+            (str, Substitution, collections.abc.Iterable),
+            'python_modules',
+            'PythonExpression')
+
         from ..utilities import normalize_to_list_of_substitutions
         self.__expression = normalize_to_list_of_substitutions(expression)
+        self.__python_modules = normalize_to_list_of_substitutions(python_modules)
 
     @classmethod
     def parse(cls, data: Iterable[SomeSubstitutionsType]):
         """Parse `PythonExpression` substitution."""
-        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 subsitution from XML,
+            # 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)
+                kwargs['python_modules'] = [module.strip() for module in modules_str.split(',')]
+        return cls, kwargs
 
     @property
     def expression(self) -> List[Substitution]:
         """Getter for expression."""
         return self.__expression
 
+    @property
+    def python_modules(self) -> List[Substitution]:
+        """Getter for expression."""
+        return self.__python_modules
+
     def describe(self) -> Text:
         """Return a description of this substitution as a string."""
-        return 'PythonExpr({})'.format(' + '.join([sub.describe() for sub in self.expression]))
+        return 'PythonExpr({}, [{}])'.format(
+            ' + '.join([sub.describe() for sub in self.expression]),
+            ', '.join([sub.describe() for sub in self.python_modules]))
 
     def perform(self, context: LaunchContext) -> Text:
         """Perform the substitution by evaluating the expression."""
         from ..utilities import perform_substitutions
-        return str(eval(perform_substitutions(context, self.expression), {}, math.__dict__))
+        module_names = [context.perform_substitution(sub) for sub in self.python_modules]
+        module_objects = [importlib.import_module(name) for name in module_names]
+        expression_locals = {}
+        for module in module_objects:
+            # For backwards compatility, we allow math definitions to be implicitly
+            # referenced in expressions, without prepending the math module name
+            # TODO: This may be removed in a future release.
+            if module.__name__ == 'math':
+                expression_locals.update(vars(module))
+
+            expression_locals[module.__name__] = module
+        return str(eval(perform_substitutions(context, self.expression), {}, expression_locals))

launch/test/launch/frontend/test_substitutions.py

@@ -206,12 +206,68 @@ def test_eval_subst():
 
 
 def test_eval_subst_of_math_expr():
+    # Math module is included by default
     subst = parse_substitution(r'$(eval "ceil(1.3)")')
     assert len(subst) == 1
     expr = subst[0]
     assert isinstance(expr, PythonExpression)
     assert '2' == expr.perform(LaunchContext())
 
+    # Do it again, with the math module explicitly given
+    subst = parse_substitution(r'$(eval "ceil(1.3)" "math")')
+    assert len(subst) == 1
+    expr = subst[0]
+    assert isinstance(expr, PythonExpression)
+    assert '2' == expr.perform(LaunchContext())
+
+    # Do it again, with the math module explicitly given and referenced in the expression
+    subst = parse_substitution(r'$(eval "math.ceil(1.3)" "math")')
+    assert len(subst) == 1
+    expr = subst[0]
+    assert isinstance(expr, PythonExpression)
+    assert '2' == expr.perform(LaunchContext())
+
+
+def test_eval_missing_module():
+    # Test with implicit math definition
+    subst = parse_substitution(r'$(eval "ceil(1.3)" "")')
+    assert len(subst) == 1
+    expr = subst[0]
+    assert isinstance(expr, PythonExpression)
+
+    # Should raise NameError since it does not have math module
+    with pytest.raises(NameError):
+        assert expr.perform(LaunchContext())
+
+    # Test with explicit math definition
+    subst = parse_substitution(r'$(eval "math.ceil(1.3)" "")')
+    assert len(subst) == 1
+    expr = subst[0]
+    assert isinstance(expr, PythonExpression)
+
+    # Should raise NameError since it does not have math module
+    with pytest.raises(NameError):
+        assert expr.perform(LaunchContext())
+
+
+def test_eval_subst_multiple_modules():
+    subst = parse_substitution(
+        r'$(eval "math.isfinite(sys.getrefcount(str(\'hello world!\')))" "math, sys")')
+    assert len(subst) == 1
+    expr = subst[0]
+    assert isinstance(expr, PythonExpression)
+    assert expr.perform(LaunchContext())
+
+
+def test_eval_subst_multiple_modules_alt_syntax():
+    # Case where the module names are listed with irregular spacing
+    subst = parse_substitution(
+        r'$(eval "math.isfinite(sys.getrefcount(str(\'hello world!\')))" " math,sys ")')
+    assert len(subst) == 1
+    expr = subst[0]
+    assert isinstance(expr, PythonExpression)
+    assert expr.perform(LaunchContext())
+
 
 def expand_cmd_subs(cmd_subs: List[SomeSubstitutionsType]):
     return [perform_substitutions_without_context(x) for x in cmd_subs]

launch/test/launch/substitutions/test_path_join_substitution.py

@@ -19,7 +19,7 @@
 from launch.substitutions import PathJoinSubstitution
 
 
-def test_this_launch_file_path():
+def test_path_join():
     path = ['asd', 'bsd', 'cds']
     sub = PathJoinSubstitution(path)
     assert sub.perform(None) == os.path.join(*path)

launch/test/launch/substitutions/test_python_expression.py

@@ -0,0 +1,116 @@
+# Copyright 2022 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Tests for the PythonExpression substitution class."""
+
+from launch import LaunchContext
+from launch.substitutions import PythonExpression
+from launch.substitutions import SubstitutionFailure
+
+import pytest
+
+
+def test_python_substitution_missing_module():
+    """Check that evaluation fails if we do not pass a needed module (sys)."""
+    lc = LaunchContext()
+    expr = 'sys.getrefcount(str("hello world!"))'
+
+    subst = PythonExpression([expr])
+
+    # Should raise a NameError since it doesn't see the sys module
+    with pytest.raises(NameError):
+        subst.perform(lc)
+
+    # Test the describe() method
+    assert subst.describe() == "PythonExpr('sys.getrefcount(str(\"hello world!\"))', ['math'])"
+
+
+def test_python_substitution_no_module():
+    """Check that PythonExpression has the math module by default."""
+    lc = LaunchContext()
+    expr = 'math.ceil(1.6)'
+
+    subst = PythonExpression([expr])
+    result = subst.perform(lc)
+
+    assert result == '2'
+
+    # Test the describe() method
+    assert subst.describe() == "PythonExpr('math.ceil(1.6)', ['math'])"
+
+
+def test_python_substitution_implicit_math():
+    """Check that PythonExpression will accept math definitions implicitly."""
+    lc = LaunchContext()
+    expr = 'ceil(1.6)'
+
+    subst = PythonExpression([expr])
+    result = subst.perform(lc)
+
+    assert result == '2'
+
+    # Test the describe() method
+    assert subst.describe() == "PythonExpr('ceil(1.6)', ['math'])"
+
+
+def test_python_substitution_empty_module_list():
+    """Case where user provides empty module list."""
+    lc = LaunchContext()
+    expr = 'math.ceil(1.6)'
+
+    subst = PythonExpression([expr], [])
+
+    # Should raise a NameError since it doesn't have the math module
+    with pytest.raises(NameError):
+        subst.perform(lc)
+
+    # Test the describe() method
+    assert subst.describe() == "PythonExpr('math.ceil(1.6)', [])"
+
+
+def test_python_substitution_one_module():
+    """Evaluation while passing one module."""
+    lc = LaunchContext()
+    expr = 'sys.getrefcount(str("hello world!"))'
+
+    subst = PythonExpression([expr], ['sys'])
+    try:
+        result = subst.perform(lc)
+    except SubstitutionFailure:
+        pytest.fail('Failed to evaluate PythonExpression containing sys module.')
+
+    # A refcount should be some positive number
+    assert int(result) > 0
+
+    # Test the describe() method
+    assert subst.describe() == "PythonExpr('sys.getrefcount(str(\"hello world!\"))', ['sys'])"
+
+
+def test_python_substitution_two_modules():
+    """Evaluation while passing two modules."""
+    lc = LaunchContext()
+    expr = 'math.isfinite(sys.getrefcount(str("hello world!")))'
+
+    subst = PythonExpression([expr], ['sys', 'math'])
+    try:
+        result = subst.perform(lc)
+    except SubstitutionFailure:
+        pytest.fail('Failed to evaluate PythonExpression containing sys module.')
+
+    # The expression should evaluate to True - the refcount is finite
+    assert result
+
+    # Test the describe() method
+    assert subst.describe() ==\
+        "PythonExpr('math.isfinite(sys.getrefcount(str(\"hello world!\")))', ['sys', 'math'])"

launch/test/temporary_environment.py

@@ -40,7 +40,6 @@ def __exit__(self, t, v, tb):
 
 def sandbox_environment_variables(func):
     """Decorate a function to give it a temporary environment."""
-
     @functools.wraps(func)
     def wrapper_func(*args, **kwargs):
         with TemporaryEnvironment():