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

Complete Voronoi Map #1605

Merged
merged 27 commits into from
Oct 15, 2021
Merged

Complete Voronoi Map #1605

merged 27 commits into from
Oct 15, 2021

Conversation

dcoeurjo
Copy link
Member

@dcoeurjo dcoeurjo commented Sep 9, 2021

PR Description

New VoronoiMap class to compute the complete V\cap\mathbb{Z}^d map (aka with all co-cyclic points).

Cherrypicking from Robin Lamy (ENSIMAG), Isabelle Sivignon (GIPSA-Lab).

TODO:

  • std::vector -> std::unordered_set for the sites container

Checklist

  • Unit-test of your feature with Catch.
  • Doxygen documentation of the code completed (classes, methods, types, members...)
  • Documentation module page added or updated.
  • New entry in the ChangeLog.md added.
  • No warning raised in Debug cmake mode (otherwise, C.I. will fail).
  • All continuous integration tests pass (Github Actions & appveyor)

@dcoeurjo dcoeurjo marked this pull request as ready for review September 9, 2021 14:10
@dcoeurjo
Copy link
Member Author

dcoeurjo commented Oct 5, 2021

This one can be reviewed too

Copy link
Member

@kerautret kerautret left a comment

Choose a reason for hiding this comment

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

Looks fine, just found some small typos and edits

src/DGtal/geometry/doc/moduleVolumetric.dox Show resolved Hide resolved
src/DGtal/geometry/doc/moduleVolumetric.dox Outdated Show resolved Hide resolved
src/DGtal/geometry/doc/moduleVolumetric.dox Outdated Show resolved Hide resolved
src/DGtal/geometry/volumes/distance/VoronoiMapComplete.h Outdated Show resolved Hide resolved
src/DGtal/geometry/volumes/distance/VoronoiMapComplete.ih Outdated Show resolved Hide resolved
src/DGtal/geometry/volumes/distance/VoronoiMapComplete.ih Outdated Show resolved Hide resolved
src/DGtal/geometry/volumes/distance/VoronoiMapComplete.ih Outdated Show resolved Hide resolved
src/DGtal/geometry/volumes/distance/VoronoiMapComplete.ih Outdated Show resolved Hide resolved
src/DGtal/geometry/volumes/distance/VoronoiMapComplete.ih Outdated Show resolved Hide resolved
@kerautret kerautret mentioned this pull request Oct 14, 2021
6 tasks
@dcoeurjo
Copy link
Member Author

All good, thx @kerautret for the review

Copy link
Member

@kerautret kerautret left a comment

Choose a reason for hiding this comment

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

Looks fine 👍🏻 Thanks, Just perhaps have a look on the indent of the parameter constructor (see annote)

@dcoeurjo
Copy link
Member Author

Thx again for the review.
I agree, some indentations are a bit weird (clang formatter). Let me check the clangformat rules to see if there is something I can do about that (maybe another PR).

@kerautret
Copy link
Member

Thx again for the review.
I agree, some indentations are a bit weird (clang formatter). Let me check the clangformat rules to see if there is something I can do about that (maybe another PR).

You are welcome, yes fine that could be also done after with other classes ;)

@dcoeurjo dcoeurjo merged commit bef5972 into DGtal-team:master Oct 15, 2021
@dcoeurjo dcoeurjo deleted the VoronoiComplete branch October 15, 2021 09:17
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.

2 participants