Add no_arr and no_str parameters #38

vinnamkim · 2019-04-06T07:36:14Z

Signed-off-by: vinnamkim vinnam.kim@gmail.com

hidmic

@vinnamkim using either parameter does not show any length, but merely empty strings/arrays, which I don't think is in the spirit of what ros2/ros2cli#136 describes. Is this still a work in progress?

vinnamkim · 2019-04-15T23:47:36Z

@vinnamkim using either parameter does not show any length, but merely empty strings/arrays, which I don't think is in the spirit of what ros2/ros2cli#136 describes. Is this still a work in progress?

I missed these things. I'll update PR.

vinnamkim · 2019-04-16T15:43:24Z

@hidmic PR is updated.

hidmic · 2019-04-22T13:52:31Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

-        result += to_string(value)
+
+        if no_arr is True and any(
+                isinstance(value, t) for t in [list, tuple, array.array, numpy.ndarray]):


@vinnamkim I think that you may achieve the same behavior by just doing:

isinstance(value, (list, tuple, array.array, numpy.ndarray))

hidmic · 2019-04-22T13:53:11Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

        return r
    result = ''
    # We rely on __slots__ retaining the order of the fields in the .msg file.
    for field_name in msg.__slots__:
        value = getattr(msg, field_name)
+        field_type = msg._fields_and_field_types[field_name[1:]]


@vinnamkim consider querying for the field type only if and when it's actually necessary

In the next commit, field_type is passed to to_string() and if-statement to check no_arr is moved to to_string(). It becomes inevitable to query for the field type only if and when it's actually necessary.

hidmic · 2019-04-22T13:53:50Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

+        if no_arr is True and any(
+                isinstance(value, t) for t in [list, tuple, array.array, numpy.ndarray]):
+            d[field_name[1:]] = '<array type: <{0}>, length: <{1}>>'.format(
+                field_type, len(value))


@vinnamkim the same two previous observations apply.

hidmic

Found a few more details while trying this out locally.

hidmic · 2019-04-22T14:30:27Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

    """
    Convert a ROS message to string of comma-separated values.

    :param msg: The ROS message to convert.
    :param truncate_length: Truncate values for all message fields to this length.
        This does not truncate the list of message fields.
+    :param no_arr: Exclude array fields of the message
+    :param no_str: Exclude string fields of the message
    :returns: A string of comma-separated values representing the input message.
    """
    def to_string(val):


@vinnamkim this isn't dealing with e.g. strings in arrays.

hidmic · 2019-04-22T14:31:33Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

+        if no_arr is True and any(
+                isinstance(value, t) for t in [list, tuple, array.array, numpy.ndarray]):
+            result += '<array type: <{0}>, length: <{1}>>'.format(
+                field_type, len(value))


@vinnamkim when the type is already bounded, length information turns out to be redundant. Consider dropping it in that case.

dirk-thomas · 2019-05-08T21:18:49Z

@vinnamkim Are you still planning to work on this PR?

vinnamkim · 2019-05-24T07:36:40Z

@hidmic @dirk-thomas
Sorry for the late response, I updated PR.

hidmic · 2019-06-10T15:20:29Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

    """
    Convert a ROS message to a YAML string.

    :param msg: The ROS message to convert.
    :param truncate_length: Truncate values for all message fields to this length.
        This does not truncate the list of message fields.
+    :param no_arr: Exclude array fields of the message
+    :param no_str: Exclude string fields of the message


@vinnamkim missing period at the end of the sentence.

hidmic · 2019-06-10T15:20:34Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

    """
    Convert a ROS message to a YAML string.

    :param msg: The ROS message to convert.
    :param truncate_length: Truncate values for all message fields to this length.
        This does not truncate the list of message fields.
+    :param no_arr: Exclude array fields of the message


@vinnamkim missing period at the end of the sentence.

hidmic · 2019-06-10T15:20:45Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

    """
    Convert a ROS message to string of comma-separated values.

    :param msg: The ROS message to convert.
    :param truncate_length: Truncate values for all message fields to this length.
        This does not truncate the list of message fields.
+    :param no_arr: Exclude array fields of the message


@vinnamkim missing period at the end of the sentence.

hidmic · 2019-06-10T15:20:51Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

    """
    Convert a ROS message to string of comma-separated values.

    :param msg: The ROS message to convert.
    :param truncate_length: Truncate values for all message fields to this length.
        This does not truncate the list of message fields.
+    :param no_arr: Exclude array fields of the message
+    :param no_str: Exclude string fields of the message


@vinnamkim missing period at the end of the sentence.

hidmic · 2019-06-10T15:21:45Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

-                    break
-                r += to_string(v)
+            if no_arr is True and field_type is not None:
+                if(callable(has_maximum_size) and has_maximum_size()):


@vinnamkim why the outer parenthesis?

hidmic · 2019-06-10T15:23:46Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

    """
    Convert a ROS message to an OrderedDict.

    :param msg: The ROS message to convert.
    :param truncate_length: Truncate values for all message fields to this length.
        This does not truncate the list of fields (ie. the dictionary keys).
+    :param no_arr: Exclude array fields of the message


@vinnamkim missing period at the end of the sentence.

hidmic · 2019-06-10T15:23:51Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

    """
    Convert a ROS message to an OrderedDict.

    :param msg: The ROS message to convert.
    :param truncate_length: Truncate values for all message fields to this length.
        This does not truncate the list of fields (ie. the dictionary keys).
+    :param no_arr: Exclude array fields of the message
+    :param no_str: Exclude string fields of the message


@vinnamkim missing period at the end of the sentence.

hidmic · 2019-06-10T15:36:26Z

rosidl_runtime_py/rosidl_runtime_py/convert.py

            value = value[:truncate_length] + '...'
    elif isinstance(value, (list, tuple, array.array, numpy.ndarray)):
        # Since arrays and ndarrays can't contain mixed types convert to list
        typename = tuple if isinstance(value, tuple) else list
-        if truncate_length is not None and len(value) > truncate_length:
+        if no_arr is True and field_type is not None:
+            if(callable(has_maximum_size) and has_maximum_size()):


@vinnamkim meta: I think you can derive both field_type and maximum_size from the value type, but that doesn't do much to the actual functionality. @dirk-thomas I'd like to know what you think.

I guess field_type is well defined in _fields_and_field_types of the parent of the value. It would be more complicated to derive field_type from the value directly.

vinnamkim · 2019-06-21T14:46:30Z

@hidmic updated PR with your comments.

hidmic

LGTM but for one detail. Arrays show like e.g.

'<array type: <uint64[3]>>'

Bounded sequences show like e.g.:

'<array type: <sequence<uint64, 3>>>'

Unbounded sequences show like e.g.:

 '<array type: <sequence<uint64>>, length: <3>>'

Thus, I wonder if the array type prefix isn't somewhat confusing when dealing with sequences.

hidmic · 2019-06-28T13:17:54Z

@jacobperron thoughts about that? If you think it's fine, let's just merge this PR and ros2/ros2cli#216.

jacobperron · 2019-07-01T23:03:29Z

I think it would be nice if the first part of the output would distinguish between array and sequence. For example, we could add an extra case if the field_type is a sequence and get output like the following:

# Array
'<array type: <uint64[3]>>'
# Sequence
'<sequence type: <uint64[3]>>'
# Unbounded sequence
'<sequence type: <uint64>, length: <3>>'

Aside: can we drop the angle brackets for the type and length? E.g.

# Array
'<array type: uint64[3]>'`
# Unbouded
'<sequence type: uint64, length: 3>'

vinnamkim · 2019-07-06T05:05:20Z

@hidmic @jacobperron Updated PR with fixing the prefix to distinguish among array, sequence, and unbounded sequence. --no-arr is working now as follows.

array: '<array type: int32[5]>'
sequence: '<sequence type: int32[5], length: 4>'
unbounded_sequence: '<sequence type: int32, length: 3>'
pose_array: '<array type: geometry_msgs/msg/Pose[5]>'
pose_sequence: '<sequence type: geometry_msgs/msg/Pose[5], length: 4>'
pose_unbounded_sequence: '<sequence type: geometry_msgs/msg/Pose, length: 3>'

jacobperron

LGTM. The PR needs a rebase and then I'll trigger CI.

Thank you for iterating with us!

- For the issue : ros2/ros2cli#136 Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

vinnamkim · 2019-07-08T22:44:03Z

LGTM. The PR needs a rebase and then I'll trigger CI.

Thank you for iterating with us!

Rebased PR. Thank you.

jacobperron · 2019-07-09T00:40:50Z

CI up to rosidl_runtime_py and CLI packages (includes ros2/ros2cli#216):

Linux
Linux-aarch64
macOS
Windows

Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

jacobperron · 2019-07-09T03:10:53Z

Linux
aarch64
macOS
Windows

jacobperron · 2019-07-09T16:29:15Z

@vinnamkim Thanks for the feature!

rosidl_runtime_py/rosidl_runtime_py/convert.py

* Add no_arr and no_str params - For the issue : ros2/ros2cli#136 Signed-off-by: vinnamkim <vinnam.kim@gmail.com> * Fix formats on --noarr and --nostr Signed-off-by: vinnamkim <vinnam.kim@gmail.com> * Fix review Signed-off-by: vinnamkim <vinnam.kim@gmail.com> * Fix comments and remove outer parenthesis Signed-off-by: vinnamkim <vinnam.kim@gmail.com> * Distinguish between array, sequence and unbounded sequence Signed-off-by: vinnamkim <vinnam.kim@gmail.com> * Fix test failure Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

tfoote added the in review Waiting for review (Kanban column) label Apr 6, 2019

vinnamkim mentioned this pull request Apr 6, 2019

Add no_str and no_arr options for ros2 topic echo ros2/ros2cli#216

Merged

hidmic reviewed Apr 15, 2019

View reviewed changes

hidmic requested changes Apr 22, 2019

View reviewed changes

hidmic reviewed Apr 22, 2019

View reviewed changes

jacobperron added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Apr 25, 2019

hidmic reviewed Jun 10, 2019

View reviewed changes

hidmic added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jun 24, 2019

hidmic approved these changes Jun 24, 2019

View reviewed changes

jacobperron reviewed Jul 8, 2019

View reviewed changes

vinnamkim added 4 commits July 9, 2019 07:33

Add no_arr and no_str params

e4fdb92

- For the issue : ros2/ros2cli#136 Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

Fix formats on --noarr and --nostr

346a37c

Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

Fix review

a616e6c

Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

Fix comments and remove outer parenthesis

c9c1bdd

Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

vinnamkim force-pushed the add_no_str_no_arr branch from 891f902 to 4322ed2 Compare July 8, 2019 22:38

Distinguish between array, sequence and unbounded sequence

dda23d4

Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

vinnamkim force-pushed the add_no_str_no_arr branch from 4322ed2 to dda23d4 Compare July 8, 2019 22:41

Fix test failure

6220c88

Signed-off-by: vinnamkim <vinnam.kim@gmail.com>

jacobperron merged commit 3a1bb97 into ros2:master Jul 9, 2019

dirk-thomas reviewed Jul 24, 2019

View reviewed changes

rosidl_runtime_py/rosidl_runtime_py/convert.py Show resolved Hide resolved

jacobperron mentioned this pull request Aug 19, 2019

Require keyword arguments be passed by name #80

Merged

Add no_arr and no_str parameters #38

Add no_arr and no_str parameters #38

Conversation

vinnamkim commented Apr 6, 2019 • edited by jacobperron Loading

hidmic left a comment

Choose a reason for hiding this comment

vinnamkim commented Apr 15, 2019

vinnamkim commented Apr 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinnamkim May 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dirk-thomas commented May 8, 2019

vinnamkim commented May 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinnamkim Jun 21, 2019 • edited Loading

Choose a reason for hiding this comment

vinnamkim commented Jun 21, 2019

hidmic left a comment

Choose a reason for hiding this comment

hidmic commented Jun 28, 2019 • edited Loading

jacobperron commented Jul 1, 2019 • edited Loading

vinnamkim commented Jul 6, 2019

jacobperron left a comment • edited Loading

Choose a reason for hiding this comment

vinnamkim commented Jul 8, 2019

jacobperron commented Jul 9, 2019 • edited Loading

jacobperron commented Jul 9, 2019

jacobperron commented Jul 9, 2019

vinnamkim commented Apr 6, 2019 •

edited by jacobperron

Loading

vinnamkim May 24, 2019 •

edited

Loading

vinnamkim Jun 21, 2019 •

edited

Loading

hidmic commented Jun 28, 2019 •

edited

Loading

jacobperron commented Jul 1, 2019 •

edited

Loading

jacobperron left a comment •

edited

Loading

jacobperron commented Jul 9, 2019 •

edited

Loading