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

ply-compatible float64 types #323

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Tofull
Copy link

@Tofull Tofull commented Jan 13, 2022

Description

.ply files were not written with "double" coordinates even though the xyz coordinates columns were in float64 format.
I was dealing with ply files in geospatial context and required to save a .ply file. I hit this issue, so here is the PR.

This seems to have been a pain for the community:

Proposed changes

  • make float64 compatible with ply files (written ply files with float64 (double) coordinates can be opened by cloudcompare)

  • make property_format mapping better (written ply files can now exploit all the scalar data types a .ply file can support)
    image

  • try downcasting column if column dtype is not compatible with PLY specification

  • fix most of flake8 issues (1 remaining)

  • backward compatible with Add support for bool type in PLY files #309

References :

This makes columns like `voxel_n` (int_64) being downcasted to the smallest numerical dtype possible.
With the data used in tests/integration/test_core_class.py::test_split_on, this column can be downcasted to int8, which is ply compatible.
@Tofull
Copy link
Author

Tofull commented Jan 13, 2022

Was wondering if the downcast propagation should be done on a copy of the points dataframe instead of the points dataframe itself.
If true, we should add points = points.copy() at the beginning of pyntcloud.io.ply.write_ply function.

@Tofull Tofull marked this pull request as ready for review January 13, 2022 18:23
@daavoo daavoo self-requested a review April 15, 2022 06:48
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.

1 participant