map Arrays to numpy.ndarray() and Sequences to array.array() #35

dirk-thomas · 2019-03-25T23:30:26Z

Implements the latest version of the design article describing the type mapping for Python (including the latest change ros2/design#217).

Linux
Linux-aarch64
macOS
Windows

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

dirk-thomas · 2019-03-27T05:06:37Z

Linux
Linux-aarch64
macOS
Windows

Ready for review.

dirk-thomas · 2019-03-27T05:06:54Z

A great example of the performance improvement this change provides is to run the following two commands:

ros2 run image_tools cam2image -x 1280 -y 720
ros2 topic echo /image

Without this patch the Python CLI takes about 600-700ms to convert the incoming ROS message into a Python object. As a consequence only every 15th-20th frames is being shown and the command line tool uses 100% of one CPU core.

With the patch the conversion time is reduced by two orders of magnitude to about 7-8ms and the command line tool can print information for every image message without utilizing a full CPU core. Also the memory usage is reduced for the Python tool since an array.array uses only roughly the size of the underlaying C array (rather than a Python list with Python int items).

kervel · 2019-03-27T12:32:39Z

Great, i think quite a few people would be helped by this (it would make the python API useable for us at least). See also ros2/ros2#509 and various threads on discourse.

mjcarroll · 2019-03-27T15:49:56Z

rosidl_generator_py/resource/_msg.py.em

@@ -18,6 +19,52 @@ from rosidl_parser.definition import Sequence
 from rosidl_parser.definition import String
 from rosidl_parser.definition import WString
 }@
+@#<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<


Would you consider using a different delimiter here? These look a lot like git conflict markers.

This kind of delimiter already exists in numerous em templates throughout the various generators. So for this PR I would rather stick with it. But a set of follow up PRs could replace them with whatever else is preferred.

Ah, sorry, I guess I hadn't noticed it before. It really jumped out at me this time. I'm good with it staying, then.

hidmic

LGTM but for tiny comments and pending green CI

rosidl_generator_py/resource/_msg.py.em

rosidl_generator_py/test/test_interfaces.py

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

…ray() Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

dirk-thomas · 2019-03-27T17:25:52Z

pending green CI

@hidmic CI builds have already been run and are referenced in the comment above.

hidmic · 2019-03-27T17:32:37Z

Oh, I see now that test failures are unrelated. LGTM then.

mjcarroll

LGTM, sorry for long review, was testing it out locally 👍

CI failures are unrelated to this change.

dirk-thomas · 2019-03-27T19:23:51Z

Merging without squashing to keep the commits separate for easier history reading. Since these changes won't be backported the separated commits shouldn't be a problem.

dirk-thomas · 2019-03-28T16:43:21Z

Addressed compiler warnings about unused variables in release mode in a follow up commit: 4f11d87.

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update tests Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> simplify logic by printing member type directly Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> move dict construction to rosidl_runtime_py Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add utility to import complex message and add support for nested array in set_message Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update tests and remove coverage for dict to avoid circular dependency Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> more descriptive variable name Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> refactor import logic according to ros2#35 Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> move imports to top of file Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> string maximum size doesn't need quoting or str conversion Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update docblock Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> slot_types_dict_from_message -> get_message_slot_types Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> check Abstrct type instead of 'list' Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> use NestedType Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add conditional is namespaces is an empty list Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> one liner Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> extend test suite to cover NesteTypes of NamespacedTypes Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> single message fixture to use Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> use NestedType Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add coverage for static array of nested messages Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> restore old API Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> SLOT_TYPES improvement from dirk-thomas Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> resolve conflicts Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

* store types as constant and return ordered dict Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update tests Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> simplify logic by printing member type directly Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> move dict construction to rosidl_runtime_py Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add utility to import complex message and add support for nested array in set_message Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update tests and remove coverage for dict to avoid circular dependency Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> more descriptive variable name Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> refactor import logic according to #35 Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> move imports to top of file Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> string maximum size doesn't need quoting or str conversion Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update docblock Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> slot_types_dict_from_message -> get_message_slot_types Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> check Abstrct type instead of 'list' Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> use NestedType Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add conditional is namespaces is an empty list Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> one liner Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> extend test suite to cover NesteTypes of NamespacedTypes Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> single message fixture to use Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> use NestedType Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add coverage for static array of nested messages Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> restore old API Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> SLOT_TYPES improvement from dirk-thomas Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> resolve conflicts Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * add back I100 ignore Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * use new rosidl_parser.definition types and new test interfaces Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * restore original test, add test for SLOT_TYPES Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

* store types as constant and return ordered dict Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update tests Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> simplify logic by printing member type directly Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> move dict construction to rosidl_runtime_py Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add utility to import complex message and add support for nested array in set_message Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update tests and remove coverage for dict to avoid circular dependency Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> more descriptive variable name Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> refactor import logic according to ros2/rosidl_python#35 Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> move imports to top of file Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> string maximum size doesn't need quoting or str conversion Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update docblock Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> slot_types_dict_from_message -> get_message_slot_types Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> check Abstrct type instead of 'list' Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> use NestedType Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add conditional is namespaces is an empty list Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> one liner Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> extend test suite to cover NesteTypes of NamespacedTypes Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> single message fixture to use Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> use NestedType Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> add coverage for static array of nested messages Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> restore old API Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> SLOT_TYPES improvement from dirk-thomas Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> resolve conflicts Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * add back I100 ignore Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * use new rosidl_parser.definition types and new test interfaces Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * restore original test, add test for SLOT_TYPES Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

dirk-thomas added enhancement New feature or request in progress Actively being worked on (Kanban column) labels Mar 25, 2019

dirk-thomas self-assigned this Mar 25, 2019

This was referenced Mar 25, 2019

install numpy ros2/ci#268

Merged

update tests to pass with numpy arrays ros2/rclpy#292

Merged

support array.array and numpy.ndarray field types ros2/ros2cli#211

Merged

dirk-thomas added 2 commits March 26, 2019 20:28

add additional Python module to CMake dependencies

49ad13c

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

manage unique import statement across templates

a74ed22

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

dirk-thomas force-pushed the array-sequence-mapping branch 2 times, most recently from 18df5bf to 6475261 Compare March 27, 2019 04:44

dirk-thomas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Mar 27, 2019

dirk-thomas mentioned this pull request Mar 27, 2019

image callback in python very slow ros2/ros2#509

Closed

mjcarroll reviewed Mar 27, 2019

View reviewed changes

dirk-thomas mentioned this pull request Mar 27, 2019

ROS 2 Dashing Diademata ros2/ros2#607

Closed

20 tasks

hidmic approved these changes Mar 27, 2019

View reviewed changes

rosidl_generator_py/resource/_msg.py.em Outdated Show resolved Hide resolved

rosidl_generator_py/test/test_interfaces.py Show resolved Hide resolved

dirk-thomas added 7 commits March 27, 2019 10:24

fix assertion messages

43f0fe3

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

change mapping of Arrays to numpy.ndarray() and Sequences to array.ar…

34902e6

…ray() Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

use numpy mapping in convert_from_py

d9236ce

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

use numpy mapping in convert_to_py

3d74185

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

update __eq__ to handle numpy arrays

9283718

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

add include directory for numpy headers

1355989

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

use array.array mapping in convert_to_py

739c50f

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

dirk-thomas force-pushed the array-sequence-mapping branch from 6475261 to 739c50f Compare March 27, 2019 17:24

mjcarroll approved these changes Mar 27, 2019

View reviewed changes

dirk-thomas merged commit a47c064 into master Mar 27, 2019

dirk-thomas deleted the array-sequence-mapping branch March 27, 2019 19:24

dirk-thomas removed the in review Waiting for review (Kanban column) label Mar 27, 2019

dirk-thomas mentioned this pull request Mar 27, 2019

mention changed mapping of arrays / sequences of numeric types in Python ros2/ros2_documentation#153

Merged

dirk-thomas added a commit that referenced this pull request Mar 28, 2019

fix compiler warning about unused variable in release mode (#35)

4f11d87

dirk-thomas mentioned this pull request Mar 28, 2019

fixes a failing test introduced by the switch to array.array ros2/system_tests#341

Merged

mikaelarguedas added a commit to mikaelarguedas/rosidl_python that referenced this pull request Mar 28, 2019

refactor import logic according to ros2#35

63e5d0c

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

mikaelarguedas mentioned this pull request Mar 28, 2019

store types as tuple of abstract types #33

Merged

This was referenced Apr 1, 2019

Windows debug build failing ros2/build_farmer#176

Closed

fix numpy usage for Windows debug builds #36

Merged

mikaelarguedas mentioned this pull request Apr 3, 2019

ros2 topic echo doesn't print arrays correctly ros2/ros2cli#213

Closed

dirk-thomas mentioned this pull request Apr 4, 2019

fix echo of numpy.number values #37

Merged

jacobperron mentioned this pull request Apr 15, 2019

Action client crashes "no attribute 'sequence'" ros2/examples#235

Closed

dirk-thomas mentioned this pull request Apr 25, 2019

Add capability to use ros2 param set for array types ros2/ros2cli#199

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

map Arrays to numpy.ndarray() and Sequences to array.array() #35

map Arrays to numpy.ndarray() and Sequences to array.array() #35

dirk-thomas commented Mar 25, 2019

dirk-thomas commented Mar 27, 2019 •

edited

dirk-thomas commented Mar 27, 2019

kervel commented Mar 27, 2019

mjcarroll Mar 27, 2019

dirk-thomas Mar 27, 2019

mjcarroll Mar 27, 2019

hidmic left a comment

dirk-thomas commented Mar 27, 2019

hidmic commented Mar 27, 2019

mjcarroll left a comment

dirk-thomas commented Mar 27, 2019

dirk-thomas commented Mar 28, 2019

map Arrays to numpy.ndarray() and Sequences to array.array() #35

map Arrays to numpy.ndarray() and Sequences to array.array() #35

Conversation

dirk-thomas commented Mar 25, 2019

dirk-thomas commented Mar 27, 2019 • edited

dirk-thomas commented Mar 27, 2019

kervel commented Mar 27, 2019

mjcarroll Mar 27, 2019

Choose a reason for hiding this comment

dirk-thomas Mar 27, 2019

Choose a reason for hiding this comment

mjcarroll Mar 27, 2019

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

dirk-thomas commented Mar 27, 2019

hidmic commented Mar 27, 2019

mjcarroll left a comment

Choose a reason for hiding this comment

dirk-thomas commented Mar 27, 2019

dirk-thomas commented Mar 28, 2019

dirk-thomas commented Mar 27, 2019 •

edited