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

Add container member function to DigitalSets and ImageContainers #1532

Merged
merged 1 commit into from
Nov 27, 2020

Conversation

phcerdan
Copy link
Member

@phcerdan phcerdan commented Nov 19, 2020

PR Description

container() returns a reference (might be const) to the underlying
container holding the data.
It might point to an instance of Self type, Parent type, or to a protected/private
data member, depending on the type.

Checklist

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

@dcoeurjo
Copy link
Member

👍
Shouldn't we update the Concepts as well if we rely on this ?
At this point, it would only be required for the python binding, not the actual "algorithms" using these data structures. @JacquesOlivierLachaud ?

@phcerdan
Copy link
Member Author

phcerdan commented Nov 19, 2020

Yeah, it's just to expose the container, I don't think we need to update the Concept, but I am all ears.
I could also make the pybind class a friend of the C++ class, but, I kind of prefer to not mix python in the pure C++ side.

Also, we are manually generating the wrappings, if some classes do not have container() defined, it's ok.
I added container() to all the current classes for consistency, but it's not needed for some of them (where the type itself is the container).

@phcerdan phcerdan force-pushed the add_container_member_function branch from 6c6f6af to 5998931 Compare November 19, 2020 14:25
src/DGtal/images/ImageContainerByHashTree.h Outdated Show resolved Hide resolved
src/DGtal/images/ImageContainerByHashTree.h Outdated Show resolved Hide resolved
src/DGtal/images/ImageContainerByHashTree.h Outdated Show resolved Hide resolved
src/DGtal/images/ImageContainerBySTLVector.h Show resolved Hide resolved
src/DGtal/kernel/sets/DigitalSetFromMap.h Outdated Show resolved Hide resolved
@JacquesOlivierLachaud
Copy link
Member

The problem in Travis seems unrelated to your PR.

@phcerdan phcerdan force-pushed the add_container_member_function branch 2 times, most recently from b1ac1de to ffec46f Compare November 19, 2020 18:50
`container()` returns a reference (might be const) to the underlying
container holding the data.
It might point to an instance of Self type, Parent type, or to a protected/private
data member, depending on the type.
@phcerdan phcerdan force-pushed the add_container_member_function branch from ffec46f to 9ea6a08 Compare November 19, 2020 18:56
@phcerdan
Copy link
Member Author

The problem in Travis seems unrelated to your PR.

The travis errors in clang seems related to doxygen, opened #1534 about it

@phcerdan phcerdan merged commit ba582e9 into DGtal-team:master Nov 27, 2020
@phcerdan phcerdan deleted the add_container_member_function branch November 27, 2020 12:12
@phcerdan phcerdan mentioned this pull request Jan 18, 2021
27 tasks
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.

None yet

3 participants