Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update point_cloud2.py to support type point xyzi #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants