Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove constructors arguments deprecated since Foxy #190

Merged
merged 5 commits into from
Oct 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 2 additions & 29 deletions launch_ros/launch_ros/actions/composable_node_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

from typing import List
from typing import Optional
import warnings

from launch.action import Action
from launch.launch_context import LaunchContext
Expand All @@ -33,10 +32,8 @@ class ComposableNodeContainer(Node):
def __init__(
self,
*,
name: Optional[SomeSubstitutionsType] = None,
namespace: Optional[SomeSubstitutionsType] = None,
node_name: Optional[SomeSubstitutionsType] = None,
node_namespace: Optional[SomeSubstitutionsType] = None,
name: SomeSubstitutionsType,
namespace: SomeSubstitutionsType,
composable_node_descriptions: Optional[List[ComposableNode]] = None,
**kwargs
) -> None:
Expand All @@ -46,35 +43,11 @@ def __init__(
Most arguments are forwarded to :class:`launch_ros.actions.Node`, so see the documentation
of that class for further details.

.. deprecated:: Foxy
Parameters `node_name` and `node_namespace` are deprecated.
Use `name` and `namespace` instead.

:param: name the name of the node, mandatory for full container node name resolution
:param: namespace the ROS namespace for this Node, mandatory for full container node
name resolution
:param: node_name (DEPRECATED) the name of the node, mandatory for full container node
name resolution
:param: node_namespace (DEPRECATED) the ros namespace for this Node, mandatory for full
container node name resolution
:param composable_node_descriptions: optional descriptions of composable nodes to be loaded
"""
if node_name is not None:
warnings.warn("The parameter 'node_name' is deprecated, use 'name' instead")
if name is not None:
raise RuntimeError(
"Passing both 'node_name' and 'name' parameters. Only use 'name'."
)
name = node_name
if node_namespace is not None:
warnings.warn("The parameter 'node_namespace' is deprecated, use 'namespace' instead")
if namespace is not None:
raise RuntimeError(
"Passing both 'node_namespace' and 'namespace' parameters. "
"Only use 'namespace'."
)
namespace = node_namespace

super().__init__(name=name, namespace=namespace, **kwargs)
self.__composable_node_descriptions = composable_node_descriptions

Expand Down
18 changes: 1 addition & 17 deletions launch_ros/launch_ros/actions/lifecycle_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from typing import cast
from typing import List
from typing import Optional
import warnings

import launch
from launch import SomeSubstitutionsType
Expand All @@ -42,9 +41,8 @@ class LifecycleNode(Node):
def __init__(
self,
*,
name: Optional[SomeSubstitutionsType] = None,
name: SomeSubstitutionsType,
namespace: SomeSubstitutionsType,
node_name: Optional[SomeSubstitutionsType] = None,
**kwargs
) -> None:
"""
Expand All @@ -70,24 +68,10 @@ def __init__(
one, or even all lifecycle nodes, and it requests the targeted nodes
to change state, see its documentation for more details.

.. deprecated:: Foxy
The parameter `node_name` is deprecated, use `name` instead.

:param name: The name of the lifecycle node.
Although it defaults to None it is a required parameter and the default will be removed
in a future release.
:param node_name: (DEPRECATED) The name fo the lifecycle node.
"""
if node_name is not None:
warnings.warn("The parameter 'node_name' is deprecated, use 'name' instead")
if name is not None:
raise RuntimeError(
"Passing both 'node_name' and 'name' parameters. Only use 'name'."
)
name = node_name
# TODO(jacobperron): Remove default value and this check when deprecated API is removed
if name is None:
raise RuntimeError("'name' must not be None.'")
super().__init__(name=name, namespace=namespace, **kwargs)
self.__logger = launch.logging.get_logger(__name__)
self.__rclpy_subscription = None
Expand Down
44 changes: 0 additions & 44 deletions launch_ros/launch_ros/actions/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
from typing import Tuple # noqa: F401
from typing import Union

import warnings

from launch.action import Action
from launch.actions import ExecuteProcess
from launch.frontend import Entity
Expand Down Expand Up @@ -72,12 +70,9 @@ class Node(ExecuteProcess):
def __init__(
self, *,
executable: Optional[SomeSubstitutionsType] = None,
jacobperron marked this conversation as resolved.
Show resolved Hide resolved
node_executable: Optional[SomeSubstitutionsType] = None,
package: Optional[SomeSubstitutionsType] = None,
name: Optional[SomeSubstitutionsType] = None,
namespace: Optional[SomeSubstitutionsType] = None,
node_name: Optional[SomeSubstitutionsType] = None,
node_namespace: SomeSubstitutionsType = None,
exec_name: Optional[SomeSubstitutionsType] = None,
parameters: Optional[SomeParameters] = None,
remappings: Optional[SomeRemapRules] = None,
Expand Down Expand Up @@ -131,39 +126,19 @@ def __init__(
passed in in order to the node (where the last definition of a
parameter takes effect).

.. deprecated:: Foxy
Parameters `node_executable`, `node_name`, and `node_namespace` are deprecated.
Use `executable`, `name`, and `namespace` instead.

:param: executable the name of the executable to find if a package
is provided or otherwise a path to the executable to run.
:param: node_executable (DEPRECATED) the name of the executable to find if a package
is provided or otherwise a path to the executable to run.
:param: package the package in which the node executable can be found
:param: name the name of the node
:param: namespace the ROS namespace for this Node
:param: exec_name the label used to represent the process.
Defaults to the basename of node executable.
:param: node_name (DEPRECATED) the name of the node
:param: node_namespace (DEPRECATED) the ros namespace for this Node
:param: parameters list of names of yaml files with parameter rules,
or dictionaries of parameters.
:param: remappings ordered list of 'to' and 'from' string pairs to be
passed to the node as ROS remapping rules
:param: arguments list of extra arguments for the node
"""
if node_executable is not None:
warnings.warn(
"The parameter 'node_executable' is deprecated, use 'executable' instead",
stacklevel=2
)
if executable is not None:
raise RuntimeError(
"Passing both 'node_executable' and 'executable' parameters. "
"Only use 'executable'"
)
executable = node_executable

if package is not None:
cmd = [ExecutableInPackage(package=package, executable=executable)]
else:
Expand All @@ -172,28 +147,9 @@ def __init__(
# Reserve space for ros specific arguments.
# The substitutions will get expanded when the action is executed.
cmd += ['--ros-args'] # Prepend ros specific arguments with --ros-args flag
if node_name is not None:
warnings.warn(
"The parameter 'node_name' is deprecated, use 'name' instead",
stacklevel=2)
if name is not None:
raise RuntimeError(
"Passing both 'node_name' and 'name' parameters. Only use 'name'."
)
cmd += ['-r', LocalSubstitution(
"ros_specific_arguments['name']", description='node name')]
name = node_name
if name is not None:
cmd += ['-r', LocalSubstitution(
"ros_specific_arguments['name']", description='node name')]
if node_namespace:
warnings.warn("The parameter 'node_namespace' is deprecated, use 'namespace' instead")
if namespace:
raise RuntimeError(
"Passing both 'node_namespace' and 'namespace' parameters. "
"Only use 'namespace'."
)
namespace = node_namespace
if parameters is not None:
ensure_argument_type(parameters, (list), 'parameters', 'Node')
# All elements in the list are paths to files with parameters (or substitutions that
Expand Down
36 changes: 0 additions & 36 deletions launch_ros/launch_ros/descriptions/composable_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

from typing import List
from typing import Optional
import warnings

from launch.some_substitutions_type import SomeSubstitutionsType
from launch.substitution import Substitution
Expand All @@ -39,56 +38,21 @@ def __init__(
plugin: Optional[SomeSubstitutionsType] = None,
name: Optional[SomeSubstitutionsType] = None,
namespace: Optional[SomeSubstitutionsType] = None,
node_plugin: Optional[SomeSubstitutionsType] = None,
node_name: Optional[SomeSubstitutionsType] = None,
node_namespace: Optional[SomeSubstitutionsType] = None,
parameters: Optional[SomeParameters] = None,
remappings: Optional[SomeRemapRules] = None,
extra_arguments: Optional[SomeParameters] = None,
) -> None:
"""
Initialize a ComposableNode description.

.. deprecated:: Foxy
Parameters `node_plugin`, `node_name`, and `node_namespace` are deprecated.
Use `plugin`, `name`, and `namespace` instead.

:param package: name of the ROS package the node plugin lives in
:param plugin: name of the plugin to be loaded
:param name: name to give to the ROS node
:param namespace: namespace to give to the ROS node
:param node_plugin: (DEPRECATED) name of the plugin to be loaded
:param node_name: (DEPRECATED) name the node should have
:param node_namespace: (DEPRECATED) namespace the node should create topics/services/etc in
:param parameters: list of either paths to yaml files or dictionaries of parameters
:param remappings: list of from/to pairs for remapping names
:param extra_arguments: container specific arguments to be passed to the loaded node
"""
if node_plugin is not None:
warnings.warn("The parameter 'node_plugin' is deprecated, use 'plugin' instead")
if plugin is not None:
raise RuntimeError(
"Passing both 'node_plugin' and 'plugin' parameters. Only use 'plugin'."
)
plugin = node_plugin
if plugin is None:
raise RuntimeError("The 'plugin' parameter is required")
if node_name is not None:
warnings.warn("The parameter 'node_name' is deprecated, use 'name' instead")
if name is not None:
raise RuntimeError(
"Passing both 'node_name' and 'name' parameters. Only use 'name'."
)
name = node_name
if node_namespace is not None:
warnings.warn("The parameter 'node_namespace' is deprecated, use 'namespace' instead")
if namespace is not None:
raise RuntimeError(
"Passing both 'node_namespace' and 'namespace' parameters. "
"Only use 'namespace'."
)
namespace = node_namespace

self.__package = normalize_to_list_of_substitutions(package)
self.__node_plugin = normalize_to_list_of_substitutions(plugin)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ def test_lifecycle_node_constructor():
executable='bsd',
name='my_node',
)
# Construction without name
# TODO(ivanpauno): This should raise TypeError.
with pytest.raises(RuntimeError):
with pytest.raises(TypeError):
LifecycleNode(
package='asd',
executable='bsd',
Expand Down
50 changes: 0 additions & 50 deletions test_launch_ros/test/test_launch_ros/actions/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import pathlib
from typing import List
import unittest
import warnings

from launch import LaunchContext
from launch import LaunchDescription
Expand Down Expand Up @@ -250,55 +249,6 @@ def test_create_node_with_invalid_parameters(self):
)
self._assert_launch_no_errors([node_action])

def test_deprecated_node_parameters(self):
"""Test that using deprecated parameters will warn/error."""
# Note that the following tests are expected to break when the deprecated
# parameters are eventually removed and should be updated accordingly.
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
node_action = launch_ros.actions.Node(
package='demo_nodes_py', node_executable='talker_qos', output='screen',
node_name='my_node', node_namespace='my_ns'
)
self._assert_launch_no_errors([node_action])
self.assertEqual(len(w), 3)
self.assertTrue(issubclass(w[0].category, UserWarning))
self.assertTrue(issubclass(w[1].category, UserWarning))
self.assertTrue(issubclass(w[2].category, UserWarning))

# Providing both 'node_executable' and 'executable' should throw
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
with self.assertRaises(RuntimeError):
launch_ros.actions.Node(
package='demo_nodes_py', node_executable='talker_qos', executable='talker_qos',
output='screen', name='my_node', namespace='my_ns'
)
self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, UserWarning))

# Providing both 'node_name' and 'name' should throw
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
with self.assertRaises(RuntimeError):
launch_ros.actions.Node(
package='demo_nodes_py', executable='talker_qos', output='screen',
node_name='my_node', name='my_node', namespace='my_ns'
)
self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, UserWarning))

# Providing both 'node_namespace' and 'namespace' should throw
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
with self.assertRaises(RuntimeError):
launch_ros.actions.Node(
package='demo_nodes_py', executable='talker_qos', output='screen',
name='my_node', node_namespace='my_ns', namespace='my_ns'
)
self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, UserWarning))

def test_launch_node_with_invalid_parameter_dicts(self):
"""Test launching a node with invalid parameter dicts."""
# Substitutions aren't expanded until the node action is executed, at which time a type
Expand Down