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

Add support for bool type in PLY files #309

Closed
Nicholas-Autio-Mitchell opened this issue Sep 30, 2021 · 5 comments
Closed

Add support for bool type in PLY files #309

Nicholas-Autio-Mitchell opened this issue Sep 30, 2021 · 5 comments

Comments

@Nicholas-Autio-Mitchell
Copy link
Contributor

Is your feature request related to a problem? Please describe.

PyntCloud.from_file() fails if the PLY contains a bool type, which occurs here in io/ply.py.

This is because there is no mapping for bool types in the ply_dtypes dictionary:

ply_dtypes = dict([
    (b'bool', '?')       # feature request to simply add this line
    (b'int8', 'i1'),
    (b'char', 'i1'),
    (b'uint8', 'u1'),
    (b'uchar', 'b1'),
    (b'uchar', 'u1'),
    (b'int16', 'i2'),
    (b'short', 'i2'),
    (b'uint16', 'u2'),
    (b'ushort', 'u2'),
    (b'int32', 'i4'),
    (b'int', 'i4'),
    (b'uint32', 'u4'),
    (b'uint', 'u4'),
    (b'float32', 'f4'),
    (b'float', 'f4'),
    (b'float64', 'f8'),
    (b'double', 'f8')
])

The mapping of '?' to bool is shown here under "one-character strings".

Example:

In [1]: import numpy as np

In [2]: np.dtype('?')
Out[3]: dtype('bool')

Describe alternatives you've considered
Finding an alternative to using pyntcloud for PLY files 🤷

Additional context
bool is a known type for NumPy of course, so it makes sense to support it, as the basic form of PyntCloud objects holds np.ndarrays. It is not specifically mentioned in the original PLY specification (as far as I can see), but supporting PLY files that contain bool fields wouldn't break anything from a user's perspective.

@daavoo
Copy link
Owner

daavoo commented Nov 9, 2021

I guess that the main issue here would be that pyntcloud could be potentially reading/writing PLY files that are not valid in any other point cloud software (or at least not those following the official PLY specification).

Probably using npz format would be the way to go when looking for full NumPy compatibility.

@Nicholas-Autio-Mitchell
Copy link
Contributor Author

Nicholas-Autio-Mitchell commented Nov 9, 2021 via email

@Nicholas-Autio-Mitchell
Copy link
Contributor Author

@daavoo - if you don't want to include this feature (behind a flag or otherwise), shall we close this issue?

@daavoo
Copy link
Owner

daavoo commented Jan 3, 2022

@daavoo - if you don't want to include this feature (behind a flag or otherwise), shall we close this issue?

I have no strong opinion @nicholas-mitchell . If someone sends P.R. and makes sense, (i.e. doesn't break default behavior) I would happily merge

@Nicholas-Autio-Mitchell
Copy link
Contributor Author

@daavoo I have opened a PR: #321

Will close this issue and continue any discussion in the PR.

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

No branches or pull requests

2 participants