Skip to content

STYLE: Remove ITKv3 Registration examples#813

Merged
hjmjohnson merged 10 commits intoInsightSoftwareConsortium:masterfrom
hjmjohnson:remove-ITKv3-registration-examples
May 6, 2019
Merged

STYLE: Remove ITKv3 Registration examples#813
hjmjohnson merged 10 commits intoInsightSoftwareConsortium:masterfrom
hjmjohnson:remove-ITKv3-registration-examples

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Remove redundancy for ITKv3 examples with respect to ITKv4.

As ITKv5 does not support ITKv3 registration any longer, completely remove the examples.

PR Checklist

hjmjohnson added 6 commits May 2, 2019 14:02
ITKv5 explicitly removed ability to compile ITKV3_COMPATIBILTY
in 8cd2517 on 2017-12-09.

ITKV3_COMPATIBILITY option is restored in order to
make a more complete testing and removal of ITKv3
examples.
These python examples have been updated in the
Examples/RegistrationITKv4 directory.
The ITKv4 directory had many documentation updates
and ITKv5 improvements performed on the examples.

This patch compared each of the RegistrationITKv3
files with the same named file in the RegistrationITKv4
directory to backport code style and documentation changes.

The primary purpose of this exercise was to identify
where the example in the RegistrationITKv3 directory
was identical to the RegistrationITKv4 directory.

A future patchset will remove the duplicated files.
These ITKv3 examples are exact duplicates of the
ITKv4 examples.  Remove the rundunancy.

```bash
for i in *; do
  diff $i ../RegistrationITKv4/$i > /dev/null 2>&1
  if [[ $? -eq 0 ]]; then
    git rm $i
  fi
done
```

rm 'Examples/RegistrationITKv3/BSplineWarping1.cxx'
rm 'Examples/RegistrationITKv3/BSplineWarping2.cxx'
rm 'Examples/RegistrationITKv3/ChangeInformationImageFilter.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration1.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration11.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration13.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration14.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration15.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration16.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration17.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration2.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration3.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration5.cxx'
rm 'Examples/RegistrationITKv3/DeformableRegistration9.cxx'
rm 'Examples/RegistrationITKv3/DisplacementFieldInitialization.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistration10.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistration15.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistration16.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistration17.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistration18.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistration19.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistration2.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistration20.cxx'
rm 'Examples/RegistrationITKv3/ImageRegistrationHistogramPlotter.cxx'
rm 'Examples/RegistrationITKv3/IterativeClosestPoint1.cxx'
rm 'Examples/RegistrationITKv3/IterativeClosestPoint2.cxx'
rm 'Examples/RegistrationITKv3/IterativeClosestPoint3.cxx'
rm 'Examples/RegistrationITKv3/LandmarkWarping2.cxx'
rm 'Examples/RegistrationITKv3/ModelToImageRegistration1.cxx'
rm 'Examples/RegistrationITKv3/ModelToImageRegistration2.cxx'
rm 'Examples/RegistrationITKv3/MultiResImageRegistration2.cxx'
rm 'Examples/RegistrationITKv3/MultiResImageRegistration3.cxx'
rm 'Examples/RegistrationITKv3/ThinPlateSplineWarp.cxx'
@hjmjohnson hjmjohnson requested review from blowekamp and dzenanz May 2, 2019 21:12
@hjmjohnson hjmjohnson self-assigned this May 2, 2019
@hjmjohnson
Copy link
Copy Markdown
Member Author

Based on comments in #809 we wanted to keep ITKv3 registration frameworks there for testing. After a considerable effort to minimize the duplication between ITKv3 and ITKv4 examples, I identified that the ITKv3 registration code was NOT USED FOR TESTING, and had been turned off and inaccessible since early in the ITKv5 conversion (as evidenced by lack of support for new spatial objects, and many C++11 warning oversights, or refactoring skips).

In the end I think that removing the directory is the best option, but the intermediate path to getting there will provide granular restore points if it becomes necessary to restore these outdated examples. I'd rather not squash them (which would be cleaner), but less granular for restore points.

@hjmjohnson hjmjohnson requested a review from thewtex May 2, 2019 21:18
@blowekamp
Copy link
Copy Markdown
Member

I got most of them working here #814

  • likely c++11 warnings
  • many of the RegistrationITKv3 examples are duplicated in the RegistrationITKv4 directory.
  • many of the RegistrationITKv4 examples still use the old registration frame work

not sure what the path forward is here.

The conditional ITKV3_COMPATIBILITY has been disabled since early in the ITKv5
conversion.  The code was not compiled, tested, or relied upon since
early in the ITKv5 process. Enforce stricter removal of that
cmake conditional.
@blowekamp
Copy link
Copy Markdown
Member

Nice work cleaning up the duplicates! I'm totally for merging that ASAP.

As for removing the reminder, I'm not on that page yet.

@hjmjohnson
Copy link
Copy Markdown
Member Author

@blowekamp Give me an hour or two. I think I have a good solution. When "BUILD_EXAMPLES" is turned on, the non-duplicated code will be built and tested (i.e. improve code coverage, and exercise old code in a way that was at one point our best practices suggestions).

I'm working through a linking issues now.

@hjmjohnson hjmjohnson force-pushed the remove-ITKv3-registration-examples branch from 7d0fcab to 6ae278b Compare May 3, 2019 15:56
Examples/RegistrationITKv3 -> Modules/Registration/Common/test/RegistrationITKv3

These deprecated examples are still useful as historical tests, but
should not be exposed to the developers as representative examples.
@hjmjohnson hjmjohnson force-pushed the remove-ITKv3-registration-examples branch from 6ae278b to 3258906 Compare May 3, 2019 18:11
hjmjohnson added 2 commits May 3, 2019 17:27
Moving the RegistrationITKv3 into the test directories now requires
RegisterRequiredFactories() to be called explicitly to register
the IO factories needed to read/write temporary files from
within the ITK internal infrastructure.
Remove the "Software Guide" comments from the
codebase to remove confusion about the intent of
these files.
@hjmjohnson
Copy link
Copy Markdown
Member Author

@blowekamp @dzenanz @thewtex This took a bit longer than expected to move the code around. I'd love to have this merged in soon so that the content in the Examples directory is only demonstrating modern best practices, and so that the long-dormant untested code starts to be tested again (at least when Examples are built).

@blowekamp
Copy link
Copy Markdown
Member

Looks great!

Could we enable the examples and the legacy testing, just to test how the CI does with these examples?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented May 6, 2019

I will add examples and ITKFEMRegistration to this build.

Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Thanks for this big cleanup!

@hjmjohnson hjmjohnson merged commit fc29340 into InsightSoftwareConsortium:master May 6, 2019
@hjmjohnson hjmjohnson deleted the remove-ITKv3-registration-examples branch October 23, 2019 13:29
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
Make the MGHIO module a shared library like the
other IO modules.

ctest -R MGH
 1/14 Test InsightSoftwareConsortium#810: ITKIOMGHInDoxygenGroup ...........   Passed    0.05 sec
 2/14 Test InsightSoftwareConsortium#811: MGHFactoryCreationTest ...........   Passed    0.09 sec
 3/14 Test InsightSoftwareConsortium#812: MGHReadImagesTest_mgz ............   Passed    4.23 sec
 4/14 Test InsightSoftwareConsortium#813: MGHReadImagesTest_mgh ............   Passed    0.68 sec
 5/14 Test InsightSoftwareConsortium#814: MGHReadImagesTest_mgh.gz .........   Passed    4.11 sec
 6/14 Test InsightSoftwareConsortium#815: itkITKIOMGHInternalTests .........   Passed    0.08 sec
 7/14 Test InsightSoftwareConsortium#816: itkITKIOMGHOriginTest ............   Passed    4.16 sec
 8/14 Test InsightSoftwareConsortium#2356: MGHIOInDoxygenGroup ..............   Passed    0.04 sec
 9/14 Test InsightSoftwareConsortium#2357: MGHFactoryCreationTest ...........   Passed    0.07 sec
10/14 Test InsightSoftwareConsortium#2358: MGHReadImagesTest_mgz ............   Passed    4.23 sec
11/14 Test InsightSoftwareConsortium#2359: MGHReadImagesTest_mgh ............   Passed    0.68 sec
12/14 Test InsightSoftwareConsortium#2360: MGHReadImagesTest_mgh.gz .........   Passed    4.13 sec
13/14 Test InsightSoftwareConsortium#2361: itkMGHIOInternalTests ............   Passed    0.07 sec
14/14 Test InsightSoftwareConsortium#2362: itkMGHIOOriginTest ...............   Passed    4.13 sec
100% tests passed, 0 tests failed out of 14
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
Make the MGHIO module a shared library like the
other IO modules.

ctest -R MGH
 1/14 Test InsightSoftwareConsortium#810: ITKIOMGHInDoxygenGroup ...........   Passed    0.05 sec
 2/14 Test InsightSoftwareConsortium#811: MGHFactoryCreationTest ...........   Passed    0.09 sec
 3/14 Test InsightSoftwareConsortium#812: MGHReadImagesTest_mgz ............   Passed    4.23 sec
 4/14 Test InsightSoftwareConsortium#813: MGHReadImagesTest_mgh ............   Passed    0.68 sec
 5/14 Test InsightSoftwareConsortium#814: MGHReadImagesTest_mgh.gz .........   Passed    4.11 sec
 6/14 Test InsightSoftwareConsortium#815: itkITKIOMGHInternalTests .........   Passed    0.08 sec
 7/14 Test InsightSoftwareConsortium#816: itkITKIOMGHOriginTest ............   Passed    4.16 sec
 8/14 Test InsightSoftwareConsortium#2356: MGHIOInDoxygenGroup ..............   Passed    0.04 sec
 9/14 Test InsightSoftwareConsortium#2357: MGHFactoryCreationTest ...........   Passed    0.07 sec
10/14 Test InsightSoftwareConsortium#2358: MGHReadImagesTest_mgz ............   Passed    4.23 sec
11/14 Test InsightSoftwareConsortium#2359: MGHReadImagesTest_mgh ............   Passed    0.68 sec
12/14 Test InsightSoftwareConsortium#2360: MGHReadImagesTest_mgh.gz .........   Passed    4.13 sec
13/14 Test InsightSoftwareConsortium#2361: itkMGHIOInternalTests ............   Passed    0.07 sec
14/14 Test InsightSoftwareConsortium#2362: itkMGHIOOriginTest ...............   Passed    4.13 sec
100% tests passed, 0 tests failed out of 14
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.

3 participants