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

Fix cubical complex functions #1390

Merged

Conversation

JacquesOlivierLachaud
Copy link
Member

@JacquesOlivierLachaud JacquesOlivierLachaud commented Jan 30, 2019

Thanks a lot for contributing to DGtal, before submitting your PR, please fill up the description and make sure that all checkboxes are checked. Please remove these lines in your PR.

PR Description

This PR fixes two issues related to CubicalComplexFunctions: issue #1362 and issue #1381 (for progrmas testCubicalComplex, testVoxelComplex and testParDirCollapse).

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, Travis C.I. will fail).
  • All continuous integration tests pass (Travis & appveyor)

Copy link
Member

@rolanddenis rolanddenis left a comment

Choose a reason for hiding this comment

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

Thx !

Can you also remove the using namespace ::DGtal::functions; of CubicalComplex::link, and fix the indentation (mixed tabs and spaces) of CubicalComplexFunctions.ih?

src/DGtal/topology/CubicalComplex.h Outdated Show resolved Hide resolved
src/DGtal/topology/CubicalComplex.h Outdated Show resolved Hide resolved
src/DGtal/topology/CubicalComplex.ih Show resolved Hide resolved
rolanddenis and others added 6 commits January 30, 2019 16:26
@phcerdan
Copy link
Member

phcerdan commented Feb 4, 2019

Should the two operators in VoxelComplex also be moved out of the namespace?

// Forward definitions.
template <typename TKSpace, typename TCellContainer>
class VoxelComplex;
namespace functions {
template <typename TKSpace, typename TCellContainer>
VoxelComplex<TKSpace, TCellContainer> &
operator-=(VoxelComplex<TKSpace, TCellContainer> &,
const VoxelComplex<TKSpace, TCellContainer> &);
template <typename TKSpace, typename TCellContainer>
VoxelComplex<TKSpace, TCellContainer>
operator-(const VoxelComplex<TKSpace, TCellContainer> &,
const VoxelComplex<TKSpace, TCellContainer> &);
} // namespace functions

I can do it as well in other branch after this gets merged.

@rolanddenis
Copy link
Member

Yes, I think we could have the same compilation issues with VoxelComplex. Please do so 🙏

@JacquesOlivierLachaud
Copy link
Member Author

So we merge and Pablo fixes VoxelComplex operators in another PR ?

@rolanddenis
Copy link
Member

Yep, mergin... Thanks for the PR!

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.

3 participants