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.

Sign up for GitHub

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

Jump to bottom

[SpatialPartitioning] Change the API of kd-tree accessors #116

Closed
wants to merge 1 commit into from
Closed

[SpatialPartitioning] Change the API of kd-tree accessors #116

wants to merge 1 commit into from

Conversation

azaleostu
Copy link
Contributor

@azaleostu azaleostu commented Oct 31, 2023 •
edited
Loading

This PR changes the API of kd-tree accessors:

  • Rename point_data to points,
  • Rename index_count and index_data to sample_count and sample_indices,
  • Remove non-const accessors of the m_nodes and m_indices containers.

Other changes:

  • Add documentation to accessors.

@azaleostu azaleostu force-pushed the kd-tree-sampling-iterator branch 2 times, most recently from ae94492 to 685b4ad Compare November 3, 2023 15:37
@azaleostu azaleostu marked this pull request as ready for review November 30, 2023 12:48
@azaleostu azaleostu force-pushed the kd-tree-sampling-iterator branch from 685b4ad to 64f3ced Compare November 30, 2023 12:49
@nmellado
Copy link
Contributor

nmellado commented Dec 3, 2023

@azaleostu please fix conflicts introduced by latest updates on master.
@ThibaultLejemble are you fine with this proposal ?

@azaleostu azaleostu force-pushed the kd-tree-sampling-iterator branch from 64f3ced to bf5dcb1 Compare December 4, 2023 10:31
@azaleostu azaleostu changed the title [SpatialPartitioning] Add kd-tree sample index iterator and query [SpatialPartitioning] Change the API of kd-tree accessors Dec 4, 2023
@azaleostu azaleostu closed this Dec 5, 2023
@azaleostu azaleostu mentioned this pull request Dec 5, 2023
Merged
@azaleostu
Copy link
Contributor Author

azaleostu commented Dec 5, 2023 •
edited
Loading

Splitting this into multiple PRs (see #122)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThibaultLejemble ThibaultLejemble ThibaultLejemble approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@azaleostu @nmellado @ThibaultLejemble