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

Inherit StatisticalOutlierRemoval<PCLPointCloud2> from FilterIndices #1663

Conversation

stefanbuettner
Copy link
Contributor

Hey all,

recently I was very happy to see that there are specializations of many algorithms for PCLPointCloud2. However, the StatisticalOutlierRemoval<PCLPointCloud2> class inherits from Filter<PCLPointCloud2> as opposed to the StatisticalOutlierRemoval<PointT> which inherits from FilterIndices<PointT>. In contrast to the Filter class, FilterIndices supports keeping the cloud organized apart from only returning an index list of the good points, of course.
This PR suggests a change from the Filter<PCLPointCloud2> base class to the FilterIndices<PCLPointCloud2> base class allowing to keep the point cloud organized and to filter only the indices.

What do you think?
I still need to squash.

Best,
Stefan

@stefanbuettner stefanbuettner force-pushed the OrganizedStatisticalOutlierRemoval branch from a6c59cf to a4e0157 Compare July 28, 2016 15:38
@taketwo
Copy link
Member

taketwo commented Jul 29, 2016

FilterIndices is a subclass of Filter, so we should not be breaking anyone's code. LGTM.

@SergioRAgostinho SergioRAgostinho added this to the pcl-1.9.0 milestone Aug 16, 2016
@taketwo taketwo merged commit 512f3a3 into PointCloudLibrary:master Sep 2, 2017
UnaNancyOwen pushed a commit to UnaNancyOwen/pcl that referenced this pull request Nov 24, 2017
…ry#1663)

Inherit from FilterIndices and implement applyFilter, move duplicate code
@taketwo taketwo changed the title Organized Statistical Outlier Removal PCLPointCloud2 Inherit StatisticalOutlierRemoval<PCLPointCloud2> from FilterIndices Sep 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants