rosidl_python icon indicating copy to clipboard operation
rosidl_python copied to clipboard

Inconsistent typecast in Python

Open Flova opened this issue 2 years ago • 1 comments

The behavior differs depending on the datatype and whether it is set via the constructor or the class constructor when generating a PointCloud2 Message in Python.

If set via the constructor, the cast is directly performed. If the data property is used instead, a check if performed whether the data already has the appropriate datatype. In addition to that, the constructor uses the property setter method, so it can just be passed through to the setter without a cast in the constructor. Bug report

Bug report

Required Info:

  • Operating System:
    • Ubuntu 20.04
  • Installation type:
    • apt
  • Version or commit hash:
    • rolling
  • Client library (if applicable):
    • rclpy

Steps to reproduce issue

  • Pass a large array.array to the PointCloud2 constructor in python
  • Time the constructor call
  • Generate the PointCloud2 names msg without setting any data
  • Set the data via msg.data = yourarray
  • Time this process

The second one is much faster, as the setter for the property skips the conversion. Implementation 1: 2.714930295944214 seconds # constructor Implementation 2: 1.379800796508789 seconds # property

Expected behavior

Both should be equally fast

Actual behavior

The second one is much faster, because the array is not casted twice.

The constructor calls the setter (redundantly) with

self.data = array.array('B', kwargs.get('data', []))

The setter itself checks if the cast is already done

def data(self, value):
        if isinstance(value, array.array):
            assert value.typecode == 'B', \
                "The 'data' array.array() must have the type code of 'B'"
            self._data = value

Additional information

Implementation considerations

Just use

self.data = kwargs.get('data', [])

I honestly don't know how to change this since the templating seems quite complex. Some advice would be appreciated.

Issue moved from https://github.com/ros2/common_interfaces/issues/176

Flova avatar Mar 20 '22 17:03 Flova

I honestly don't know how to change this since the templating seems quite complex. Some advice would be appreciated.

@Flova you could check for instance type right before this line.

hidmic avatar Apr 25 '22 14:04 hidmic