Enable Python reserved names in Action attributes (#12)

Closes #6

This enables the condition attribute of the base Action class, which is if in YAML/XML, but that's a Python language keyword.

To handle this generally, instead suffix any reserved word with _, so it looks like

executable(cmd='echo hello', if_='True')

Author: emersonknapp

launch_frontend_py/actions.py

@@ -14,13 +14,14 @@
 
 from typing import Any, Callable
 
-from .entity import Entity, is_reserved_identifier
+from .entity import Entity
+from .util import is_reserved_identifier
 
 __all__ = []
 
 
-def make_action_factory(action_name: str, **kwargs) -> Callable[..., Any]:
-    # Create a factory function for each action entity
+def _make_action_factory(action_name: str) -> Callable[..., Any]:
+    """Create a factory function that constructs an Entity of the given action type."""
     if is_reserved_identifier(action_name):
         action_name += '_'
 
@@ -36,15 +37,25 @@ def fn(*args, **kwargs):
     return fn
 
 
-def preseed_all_actions() -> None:
-    """Pre-seed all actions in the module."""
+def _preseed_all_actions() -> None:
+    """
+    Pre-seed all available actions into this module.
+
+    This is called at module load time, but will not have access to actions from plugin packages
+    (such as launch_ros) until they have been imported into the Python process.
+    """
     from launch.frontend.expose import action_parse_methods
 
     for action_name in action_parse_methods.keys():
-        make_action_factory(action_name)
+        _make_action_factory(action_name)
 
 
 def __getattr__(name: str) -> Any:
+    """
+    Dynamically create action factory functions on demand.
+
+    This is called when user `from launch_frontend_py.actions import <name>`.
+    """
     from launch.frontend.expose import action_parse_methods
     import importlib
 
@@ -55,12 +66,12 @@ def __getattr__(name: str) -> Any:
     # If not, perhaps it was exposed later or not accessed yet
     base_name = name[:-1] if name.endswith('_') else name
     if base_name in action_parse_methods:
-        return make_action_factory(name)
+        return _make_action_factory(name)
     else:
         msg = f'module {__name__} has no attribute "{name}"'
         if base_name != name:
             msg += ' (or "{base_name}")'
         raise AttributeError(msg)
 
 
-preseed_all_actions()
+_preseed_all_actions()

launch_frontend_py/entity.py

@@ -13,9 +13,7 @@
 # limitations under the License.
 
 """Module for launch_frontend_py Entity class."""
-import builtins
 from collections.abc import Iterable
-import keyword
 from typing import (
     List,
     Optional,
@@ -32,10 +30,7 @@
     is_instance_of,
 )
 
-
-def is_reserved_identifier(name: str) -> bool:
-    """Check if a name is a reserved identifier in Python."""
-    return keyword.iskeyword(name) or name in dir(builtins)
+from .util import is_reserved_identifier
 
 
 class Entity(BaseEntity):
@@ -113,6 +108,10 @@ def get_attr(  # type: ignore[override]
         See :py:meth:`launch.frontend.Entity.get_attr`.
         Does not apply type coercion, only checks if the read value is of the correct type.
         """
+        # Reserved identifiers are all suffixed with an underscore
+        if is_reserved_identifier(name):
+            name += '_'
+
         if name not in self.__attrs:
             if not optional:
                 raise AttributeError(

launch_frontend_py/util.py

@@ -0,0 +1,25 @@
+# Copyright 2025 Polymath Robotics, 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 builtins
+import keyword
+
+
+def is_reserved_identifier(name: str) -> bool:
+    """
+    Check if a name is a reserved identifier in Python.
+
+    Used to avoid naming issues in the launch DSL that overlap with Python reserved words.
+    """
+    return keyword.iskeyword(name) or name in dir(builtins)

test/launch/basic_launch.py

@@ -19,5 +19,8 @@
 def generate_launch_description():
     return launch([
         arg(name='message', default='hello world'),
+        arg(name='condition', default='True'),
         executable(cmd='echo $(var message)', output='both'),
+        executable(cmd='echo hello conditional', if_='$(var condition)', output='both'),
+        executable(cmd='echo hello not-condition', if_='$(not $(var condition))', output='both'),
     ])

test/test_frontend.py

@@ -15,35 +15,40 @@
 from pathlib import Path
 
 from launch.actions import DeclareLaunchArgument, ExecuteProcess, IncludeLaunchDescription
-from launch.launch_context import LaunchContext
 from launch.launch_description import LaunchDescription
+from launch.launch_service import LaunchService
 
 THIS_DIR = Path(__file__).parent
 
 
 def test_load_basic():
-    context = LaunchContext()
-
     include = IncludeLaunchDescription(str(THIS_DIR / 'launch' / 'basic_launch.py'))
-    print(include)
+
+    ls = LaunchService()
+    ld = LaunchDescription([include])
+    ls.include_launch_description(ld)
+
+    ls.context.launch_configurations['condition'] = 'False'
+    assert 0 == ls.run()
+
     included_entities = include.get_sub_entities()
-    print(included_entities)
     assert len(included_entities) == 1
 
     launch_desc = included_entities[0]
     assert isinstance(launch_desc, LaunchDescription)
 
     launchfile_entities = launch_desc.describe_sub_entities()
-    assert len(launchfile_entities) == 2
-
-    # The first entity is a declare()
-    should_be_declare = launchfile_entities[0]
-    assert isinstance(should_be_declare, DeclareLaunchArgument)
-    should_be_declare.visit(context)
-
-    # Second is executable()
-    should_be_exec = launchfile_entities[1]
-    assert isinstance(should_be_exec, ExecuteProcess)
-
-    should_be_exec.prepare(context)
-    assert should_be_exec.process_description.final_cmd == ['echo', 'hello world']
+    expected_types = (
+        DeclareLaunchArgument,
+        DeclareLaunchArgument,
+        ExecuteProcess,
+        ExecuteProcess,
+        ExecuteProcess,
+    )
+    assert len(launchfile_entities) == len(expected_types)
+    assert all(isinstance(e, t) for e, t in zip(launchfile_entities, expected_types))
+
+    assert launchfile_entities[2].process_description.final_cmd == ['echo', 'hello world']
+    assert launchfile_entities[3].process_description.final_cmd is None
+    assert launchfile_entities[4].process_description.final_cmd == [
+        'echo', 'hello', 'not-condition']