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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated #1765

Open
wants to merge 12 commits into
base: main2.0
Choose a base branch
from
Open

Remove deprecated #1765

wants to merge 12 commits into from

Conversation

BDoignies
Copy link
Contributor

@BDoignies BDoignies commented Mar 3, 2025 •
edited
Loading

PR Description

Remove deprecated classes and functions.

  • Questionable changes may be made to the LocalConvolutionNormalVectorEstimator test codes. See the relevant commit message for more details.
  • Some tests might be remove as the class they were used to test no longer exists (although there is a replacement)
  • Shapes::addNorm1Ball and Shapes::addNorm2Ball are marked as deprecated but this seems inconsistent because 'Shapes::removeNorm1Ball' and 'Shapes::removeNorm2Ball' are not. Furthermore, they are still widely used. I did not remove them but I can if anyone you confirm they should be removed.

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 mode.
  • All continuous integration tests pass (Github Actions)

BDoignies added 8 commits March 3, 2025 17:40
@BDoignies
Remove safe-to delete content
b2592fd
@BDoignies
remove GridCurve::initFromVector
d133c2b
@BDoignies
Fix Viewer3D material can not be assigned
9b15edc
@BDoignies
Remove deprecated LocalConvolutionNormalVectorEstimator
ffa58d9 
List of changes:
* Remove "LocalConvolutionNormalVectorEstimator" and "BasicConvolutionWeights"
* Tests using the class now uses the replacement "LocalEstomatorFromSurfelFunctorAdapter"
* Added an overload to eval() function required by some tests
* Added a function evalAll required by some tests
* Remove subtype DigitalSurfaceEmbedderWithNormalVectorEstimator::NVESurface (it was not used anywhere else) and corresponding assert.
* Remove concept check CNormalVectorEstimator as it does not correspond to the new method and was only used at a single place.
@BDoignies
Remove function marked as deprecated (but not in header)
c03c919
@BDoignies
Updated documentation to remove reference to deprecated functions and…
e8bd77c 
… classes.
@BDoignies
Update changelog entries
df63bc6
@BDoignies
Update links in ChangeLog after wrong pull request...
90c34bb
@dcoeurjo
Copy link
Member

dcoeurjo commented Mar 4, 2025

Thanks for the PR. Checking this one

@BDoignies
Copy link
Contributor Author

Hey,

The errors remains are the one from compiling DGtalTools. Before updating the "DGtalTools/main2.0" PR, could you pre-approve the changes please ?

I am not exactly sure as how to handle things differently... Otherwise, each PR should update github actions to test against the appropriate PR; but when merged the code should be removed and link the appropriate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BDoignies @dcoeurjo