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

Avoid the limitation of not being able to import rclpy C extensions at module level #417

Closed
wants to merge 1 commit into from
Closed
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
12 changes: 6 additions & 6 deletions rclpy/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ if(WIN32 AND CMAKE_BUILD_TYPE STREQUAL "Debug")
set(PYTHON_EXECUTABLE "${PYTHON_EXECUTABLE_DEBUG}")
endif()

# enables using the Python extensions from the build space for testing
file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/test_rclpy/__init__.py" "")

ament_python_install_package(${PROJECT_NAME})

# enables using the Python extensions from the build space for testing
configure_file("resource/__init__.py.in" "${PROJECT_NAME}/__init__.py" @ONLY)

function(set_properties _targetname _build_type)
set_target_properties(${_targetname} PROPERTIES
PREFIX ""
LIBRARY_OUTPUT_DIRECTORY${_build_type} "${CMAKE_CURRENT_BINARY_DIR}/test_${PROJECT_NAME}"
RUNTIME_OUTPUT_DIRECTORY${_build_type} "${CMAKE_CURRENT_BINARY_DIR}/test_${PROJECT_NAME}"
LIBRARY_OUTPUT_DIRECTORY${_build_type} "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}"
RUNTIME_OUTPUT_DIRECTORY${_build_type} "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}"
OUTPUT_NAME "_${_targetname}${PythonExtra_EXTENSION_SUFFIX}"
SUFFIX "${PythonExtra_EXTENSION_EXTENSION}")
endfunction()
Expand Down Expand Up @@ -218,7 +218,7 @@ if(BUILD_TESTING)
ament_add_pytest_test(${_test_name} ${_test_path}
PYTHON_EXECUTABLE "${PYTHON_EXECUTABLE}"
APPEND_ENV AMENT_PREFIX_PATH=${ament_index_build_path}
PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR}
ENV RCLPY_TEST_LIBRARY_DIR=${CMAKE_CURRENT_BINARY_DIR}
TIMEOUT 60
WERROR ON
)
Expand Down
3 changes: 1 addition & 2 deletions rclpy/rclpy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
from typing import TYPE_CHECKING

from rclpy.context import Context
from rclpy.impl.implementation_singleton import rclpy_implementation
from rclpy.parameter import Parameter
from rclpy.task import Future
from rclpy.utilities import get_default_context
Expand All @@ -68,8 +69,6 @@ def init(*, args: List[str] = None, context: Context = None) -> None:
(see :func:`.get_default_context`).
"""
context = get_default_context() if context is None else context
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
return rclpy_implementation.rclpy_init(args if args is not None else sys.argv, context.handle)


Expand Down
9 changes: 2 additions & 7 deletions rclpy/rclpy/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import threading
from typing import Callable

from rclpy.impl.implementation_singleton import rclpy_implementation


class Context:
"""
Expand All @@ -26,7 +28,6 @@ class Context:
"""

def __init__(self):
from rclpy.impl.implementation_singleton import rclpy_implementation
self._handle = rclpy_implementation.rclpy_create_context()
self._lock = threading.Lock()
self._callbacks = []
Expand All @@ -38,8 +39,6 @@ def handle(self):

def ok(self):
"""Check if context hasn't been shut down."""
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
with self._lock:
return rclpy_implementation.rclpy_ok(self._handle)

Expand All @@ -51,16 +50,12 @@ def _call_on_shutdown_callbacks(self):

def shutdown(self):
"""Shutdown this context."""
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
with self._lock:
rclpy_implementation.rclpy_shutdown(self._handle)
self._call_on_shutdown_callbacks()

def try_shutdown(self):
"""Shutdown this context, if not already shutdown."""
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
with self._lock:
if rclpy_implementation.rclpy_ok(self._handle):
rclpy_implementation.rclpy_shutdown(self._handle)
Expand Down
5 changes: 1 addition & 4 deletions rclpy/rclpy/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from rclpy.constants import S_TO_NS
from rclpy.context import Context
from rclpy.impl.implementation_singleton import rclpy_implementation

g_default_context = None
g_context_lock = threading.Lock()
Expand All @@ -37,8 +38,6 @@ def get_default_context(*, shutting_down=False):


def remove_ros_args(args=None):
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
return rclpy_implementation.rclpy_remove_ros_args(
args if args is not None else sys.argv)

Expand All @@ -63,8 +62,6 @@ def try_shutdown(*, context=None):


def get_rmw_implementation_identifier():
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
return rclpy_implementation.rclpy_get_rmw_implementation_identifier()


Expand Down
35 changes: 35 additions & 0 deletions rclpy/resource/__init__.py.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# -*- coding: utf-8 -*-
# generated from rclpy/resource/__init__.py.in
# keep symbol table as clean as possible by deleting all unnecessary symbols

from os import path as os_path
from pkgutil import extend_path
from sys import path as sys_path

source_path = '@CMAKE_CURRENT_SOURCE_DIR@'
sys_path.insert(0, source_path)
del sys_path

__path__ = extend_path(__path__, __name__)
del extend_path

__execfiles = []
src_init_file = os_path.join(source_path, __name__ + '.py')
if os_path.isfile(src_init_file):
__execfiles.append(src_init_file)
else:
src_init_file = os_path.join(source_path, __name__, '__init__.py')
if os_path.isfile(src_init_file):
__execfiles.append(src_init_file)
del os_path
del src_init_file
del source_path

for __execfile in __execfiles:
with open(__execfile, 'r') as __fh:
__code_object = compile(__fh.read(), __execfile, 'exec')
exec(__code_object)
del __code_object
del __fh
del __execfile
del __execfiles
15 changes: 5 additions & 10 deletions rclpy/test/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,13 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import importlib
import os
import sys

assert 'rclpy' not in sys.modules, 'rclpy should not have been imported before running tests'

# this will make the extensions load from the build folder
import rclpy.impl # noqa
import test_rclpy # noqa
# this will load rclpy from the build folder
sys.path.insert(0, os.environ.get('RCLPY_TEST_LIBRARY_DIR'))
Copy link
Contributor

@hidmic hidmic Sep 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivanpauno using that envvar is so shady... there has to be a natural way to do this. I wonder what setuptools does under the hood when installing a regular Python package.



def _custom_import(name):
return importlib.import_module(name, package='test_rclpy')


rclpy.impl._import = _custom_import
import rclpy # noqa
assert rclpy # silence pyflakes