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

Angle limit filter #591

Merged
merged 23 commits into from
Dec 16, 2024
Merged

Angle limit filter #591

merged 23 commits into from
Dec 16, 2024

Conversation

boxanm
Copy link
Collaborator

@boxanm boxanm commented Dec 6, 2024

Description

Summary:

Add a new DataPointsFilter that filters points based on if they lie inside / outside of a given spherical wedge.
The spherical wedge is defined by the lower and upper bounds of two angles in spherical coordinates.
The angles follow the spherical physics convention, as described on Wikipedia https://en.wikipedia.org/wiki/Spherical_coordinate_system

Changes and type of changes (quick overview):

  • Also improves Python bindings installation by enabling newer pybind11 versions and supporting pybind11 installed with pip
  • Pumps the minimal required cmake version to 3.15

Checklist:

Code related

  • I have made corresponding changes to the documentation
    (i.e.: function, class, script header, README.md)
  • I have commented hard-to-understand code
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally with my changes
    (Check contributing_instructions.md for local testing procedure using libpointmatcher-build-system)

PR creation related

  • My pull request base ref branch is set to the develop branch
    (the build-system won't be triggered otherwise)
  • My pull request branch is up-to-date with the develop branch
    (the build-system will reject it otherwise)

PR description related

  • I have included a quick summary of the changes
  • I have indicated the related issue's id with # <issue-id> if changes are of type fix
  • I have included a high-level list of changes and their corresponding types
    (See commit_msg_reference.md for details)

@boxanm boxanm marked this pull request as ready for review December 9, 2024 17:31
@simonpierredeschenes
Copy link
Collaborator

The code looks fine, except for the angle_min and angle_max variables which are not necessary. I also think that unit tests should be added.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
4.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@boxanm boxanm merged commit 1289540 into develop Dec 16, 2024
8 of 9 checks passed
@boxanm boxanm deleted the angle-limit-filter branch December 16, 2024 14:49
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