Skip to content

test for AddPoints() in DFPointCloud - #63

Merged
9and3 merged 2 commits into
mainfrom
test_suite_cpp_addpoint
Aug 14, 2024
Merged

test for AddPoints() in DFPointCloud#63
9and3 merged 2 commits into
mainfrom
test_suite_cpp_addpoint

Conversation

@DamienGilliard

@DamienGilliard DamienGilliard commented Aug 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Tiny PR to add a test of AddPoints() method of DFPointCloud

The two files changed are

  • the DFPointCloudTest.cc for the c++ test of the AddPoints() method
  • the test_pybind_units.py for its pybind

@DamienGilliard DamienGilliard self-assigned this Aug 13, 2024
@DamienGilliard

DamienGilliard commented Aug 13, 2024

Copy link
Copy Markdown
Collaborator Author

@9and3 I believe this PR is ready for review

@DamienGilliard
DamienGilliard requested a review from 9and3 August 13, 2024 09:42
9and3
9and3 requested changes Aug 13, 2024
View reviewed changes

@9and3 9and3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lookin good the c++ test but we are missing the python one. Can you add it plis?

@DamienGilliard

DamienGilliard commented Aug 13, 2024

Copy link
Copy Markdown
Collaborator Author

actually I separated cpp and python (maybe not the most logical). The python test has been added in #59 . I adapted the name of the PR accordingly

@9and3

9and3 commented Aug 13, 2024

Copy link
Copy Markdown
Contributor

I see.. Can you please close both of them and do an uniqu PR? Thanks!

@DamienGilliard

DamienGilliard commented Aug 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@9and3 If you agree I'll close this one and integrate it into the other one (#59 ), because the other one is clean, I do not see a reason to close it and re-open a new PR with the same changes (Or I don't understand why you ask to open a new PR ;) ):
Screenshot 2024-08-13 at 16 40 37

Let me know !
I'll do that this evening

@DamienGilliard DamienGilliard linked an issue Aug 13, 2024 that may be closed by this pull request
add_points() backend/frontend needs unit test #62
Closed
@DamienGilliard

DamienGilliard commented Aug 13, 2024

Copy link
Copy Markdown
Collaborator Author

@9and3 Here is the PR concerning only the AddPoints() method.

Sorry for the messiness with which I handled the various parallel PR...

9and3
9and3 approved these changes Aug 14, 2024
View reviewed changes

@9and3 9and3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm! 🪨

@9and3
9and3 merged commit 69ca054 into main Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@9and3 9and3 9and3 approved these changes

Assignees

@DamienGilliard DamienGilliard

Labels

backend test_suite

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add_points() backend/frontend needs unit test

2 participants

@DamienGilliard @9and3