Skip to content
This repository was archived by the owner on Dec 30, 2023. It is now read-only.

Conversation

@larsmans
Copy link
Contributor

@larsmans larsmans commented Jul 7, 2014

This is a pretty massive change to the codebase. It replaces PointCloud.{from_file,to_file} with free-standing load and save functions. To prevent excessive compile times, these functions are defined in a pure Python module, and the code is reorganized into a package to make this possible.

Also, the PointCloud constructor was pimped to take a list, array or integer argument for initialization. I found that more Pythonic than the current "construct, then fill" idiom.

The first four commits are actually the ones from #30; I wanted to avoid merge conflicts with that PR.

larsmans added 7 commits July 3, 2014 17:23
XXX the at() members of PointCloud[T] can also throw:

>>> a = np.random.randn(10000, 3).astype(np.float32)
>>> p = PointCloud()
>>> p.from_array(a)
>>> p[10000]
terminate called after throwing an instance of 'std::out_of_range'
  what():  vector::_M_range_check
Aborted

... but a bug in Cython's code generator prevents "except+" from
working with a function returning a reference.
Using pointers to work around broken C++ reference support in Cython.
operator[] seems to be completely broken in Cython.

E.g.

>>> a = np.random.randn(10000, 3).astype(np.float32)
>>> p = PointCloud()
>>> p.from_array(a)
>>> %timeit p.to_array()
10000 loops, best of 3: 23.4 µs per loop

Previously 50.8 µs per loop.
Allows splitting the big pcl.pyx module and writing parts
in pure Python to speed up compilation.
@nzjrs
Copy link
Contributor

nzjrs commented Jul 7, 2014

Looks good, I have no complaints with your API changes, nor the re-organisation.

Do you have a minimal PLY file you could commit to the repository for testing?

I'll merge this branch as it includes #30 (which looked fine too)

@larsmans
Copy link
Contributor Author

larsmans commented Jul 7, 2014

I can push a unit test that converts a PCD to PLY and back, is that ok?

@nzjrs
Copy link
Contributor

nzjrs commented Jul 7, 2014

Yeah, that would be fine.

On Mon, Jul 7, 2014 at 1:52 PM, Lars Buitinck [email protected]
wrote:

I can push a unit test that converts a PCD to PLY and back, is that ok?


Reply to this email directly or view it on GitHub
#31 (comment).

@larsmans
Copy link
Contributor Author

larsmans commented Jul 7, 2014

Added such a unit test in 4e103e8. Also changed the test to not leave clutter in /tmp.

nzjrs added a commit that referenced this pull request Jul 7, 2014
New Pythonic pcl.{load,save} functions; PLY format support
@nzjrs nzjrs merged commit 126b4cf into strawlab:master Jul 7, 2014
@nzjrs
Copy link
Contributor

nzjrs commented Jul 7, 2014

Thanks for your work!

@larsmans larsmans deleted the io branch July 7, 2014 12:10
Sirokujira pushed a commit to Sirokujira/python-pcl that referenced this pull request Mar 10, 2017
New Pythonic pcl.{load,save} functions; PLY format support
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants