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

Inconsistent typecast in Python #155

Open
Flova opened this issue Mar 20, 2022 · 1 comment
Open

Inconsistent typecast in Python #155

Flova opened this issue Mar 20, 2022 · 1 comment
Labels
backlog bug Something isn't working

Comments

@Flova
Copy link

Flova commented Mar 20, 2022

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 ros2/common_interfaces#176

@hidmic
Copy link
Contributor

hidmic commented Apr 25, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants