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

Added missing laz extension #288

Merged
merged 1 commit into from
Oct 5, 2020
Merged

Added missing laz extension #288

merged 1 commit into from
Oct 5, 2020

Conversation

SBCV
Copy link
Contributor

@SBCV SBCV commented Oct 2, 2020

The laspy library allows to read not only las but also laz files (assuming that laszip is installed)

Btw: Are you aware of the pylas / lazperf libraries?
They also allow to read las/laz files and can be installed using pip (which is more convenient than the installation of laspy and laszip)

@SBCV
Copy link
Contributor Author

SBCV commented Oct 2, 2020

I've just made a simple timing comparison and it looks like there is a drastic performance difference between laspy and pylas when reading laz files

laspy: elapsed time: 14.988102674484253
pylas: elapsed time: 0.7680809497833252

I think that difference stems from the fact that laspy calls the command line tool of laszip

Copy link
Owner

@daavoo daavoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @SBCV ! Could you please add a test case for thins kind odd files to:

def test_from_file(data_path, extension, color, mesh):

@daavoo
Copy link
Owner

daavoo commented Oct 2, 2020

Hola @SBCV !

I've just made a simple timing comparison and it looks like there is a drastic performance difference between laspy and pylas when reading laz files

laspy: elapsed time: 14.988102674484253
pylas: elapsed time: 0.7680809497833252

I think that difference stems from the fact that laspy calls the command line tool of laszip

Nice!. I did not know about the existence of pylas (I think it did not exist back when the .las reader was added). It looks like a good and more active alternative, would be nice for a P.R.

@SBCV
Copy link
Contributor Author

SBCV commented Oct 4, 2020

In order to add the requested test to test_from_file.py , I opened diamond.las with Cloudcompare and stored the imported point cloud as diamond.laz.
When running the tests the following error message is thrown:

tests/integration/io/test_from_file.py:51: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

data = PyntCloud
6 points with 10 scalar fields
0 faces in mesh
0 kdtrees
0 voxelgrids
Centroid: 0.5, 0.5, 0.5
Other attributes:
	 las_header: <class 'laspy.header.HeaderManager'>

    def assert_points_xyz(data):
        assert np.isclose(data.points['x'][0], 0.5)
        assert np.isclose(data.points['y'][0], 0)
        assert np.isclose(data.points['z'][0], 0.5)
    
>       assert str(data.points["x"].dtype) == 'float32'
E       AssertionError: assert 'float64' == 'float32'
E         - float64
E         + float32

tests/integration/io/test_from_file.py:13: AssertionError

Do you know, if it is it possible to create laz files with 32 bit floats? Or should we change the assertions?
Any recommendations?

@daavoo
Copy link
Owner

daavoo commented Oct 5, 2020

In order to add the requested test to test_from_file.py , I opened diamond.las with Cloudcompare and stored the imported point cloud as diamond.laz.
When running the tests the following error message is thrown:

tests/integration/io/test_from_file.py:51: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

data = PyntCloud
6 points with 10 scalar fields
0 faces in mesh
0 kdtrees
0 voxelgrids
Centroid: 0.5, 0.5, 0.5
Other attributes:
	 las_header: <class 'laspy.header.HeaderManager'>

    def assert_points_xyz(data):
        assert np.isclose(data.points['x'][0], 0.5)
        assert np.isclose(data.points['y'][0], 0)
        assert np.isclose(data.points['z'][0], 0.5)
    
>       assert str(data.points["x"].dtype) == 'float32'
E       AssertionError: assert 'float64' == 'float32'
E         - float64
E         + float32

tests/integration/io/test_from_file.py:13: AssertionError

Do you know, if it is it possible to create laz files with 32 bit floats? Or should we change the assertions?
Any recommendations?

I'm guessing that CloudCompare is exporting with float64:
http://www.danielgm.net/cc/forum/viewtopic.php?t=4167#p18933

I don't know if you can change the exported precission.

Rather than changing assertions, I think that the best option would be to cast the point coordinates to float32 (maybe using a default argument in order to let someone get float64 if needed):

https://github.com/daavoo/pyntcloud/blob/master/pyntcloud/io/las.py#L28

@SBCV
Copy link
Contributor Author

SBCV commented Oct 5, 2020

Rather than changing assertions, I think that the best option would be to cast the point coordinates to float32 (maybe using a default argument in order to let someone get float64 if needed):

Added default arguments for float and color. The test case for pytest runs now successfully (including color information).
Squashed the commits to keep it in a clean state.

Copy link
Owner

@daavoo daavoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @SBCV !

@daavoo daavoo merged commit d30e0de into daavoo:master Oct 5, 2020
@SBCV
Copy link
Contributor Author

SBCV commented Oct 5, 2020

Thank you for merging.

Regarding pylas / lazperf: Do think it would be better to provide an alternative to laspy or should we replace it entirely?

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.

2 participants