Skip to content

Conversation

@rancheng
Copy link

I have use your library for a long time and am very appreciate for your hard working towards advancing this field! Thanks for your contribution all the time, Awesome work!

I updated the function in point_cloud2.py to support type XYZI point cloud, when I was trying to use your code to convert into Kitti bin format.

@gstavrinos
Copy link
Collaborator

@eric-wieser Do you want me to look into it?

@eric-wieser
Copy link
Owner

This seems not particularly worth it to me. You can get what you want with numpy builtins these days:

from numpy.lib.recfunctions import structured_to_unstructured

fields = ['x', 'y', 'z', 'intensity']
xyzi_arr = structured_to_unstructured(pointcloud2_to_array(cloud_msg)[fields])

@gstavrinos
Copy link
Collaborator

Sure, but Ran's solution is more elegant.

@eric-wieser
Copy link
Owner

I'd argue more elegant would be to not combine the arrays into a flat array in the first place. xyz makes sense to combine together because they at least mean the same thing.

I can't think of a convincing use-case for an array like array([x, y, z, i], dtype=float) instead of array([(x, y, z), i], dtype=[('v', (float, 3)), ('i', float)]) - at least, not one that makes intensity more special than any other field that could appear.

@gstavrinos
Copy link
Collaborator

Agreed. I would also expect the structure of the array to be [(x,y,z), i]. I was not talking about the implementation, but rather the ease of use.

@eric-wieser
Copy link
Owner

eric-wieser commented May 5, 2020

Perhaps then what is actually missing is merge_xyz_fields which is similar to the API of merge_rgb_fields, but uses (float, 3) instead of uint32 as the combined field.

@tpet
Copy link

tpet commented May 5, 2020

To me, it seems unnecessary to bloat the code with special functions for many possible point types if a generic conversion is simple as np.stack([c[f] for f in c.dtype.names]).

@eric-wieser
Copy link
Owner

The one advantage of structured_to_unstructured is that it may prevent a copy occuring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants