ITK icon indicating copy to clipboard operation
ITK copied to clipboard

Fix and update all remote modules to adhere to new testing structure

Open mseng10 opened this issue 5 years ago • 23 comments

Description

All of ITK's remote modules must be configured to build and test cleanly using ITK version 5.1.0. In addition, all remote modules must adhere to the new automated testing structure that requires the deprecation of CircleCI, Travis, AppVeyor and Azure pipeline testing with the new implementation of GitHub Actions. Using GitHub Actions we can allow users to test their respective changes without the authorization of an administrator.

Tasks

I will be updating these modules on a per module basis, with each containing their own respective PR. For each PR, the branch to be merged with be labeled github-actions. In addition, I will be contacting @hjmjohnson about needed authorized actions.

Below is a list of tasks that must be accomplished in the following order. This list is derived from a remote module's previous PR here.

  1. Bump remote module version to 5.1 in CMakeList.txt

  2. Bump itk version to 5.1 in install_requires in the setup.py file

  3. Copy GitHub Actions configuration files from ITKModuleTemplate to specific remote module and update README to contain new GitHub Actions Badge.

  4. Make PR and monitor builds. Any issues must be fixed or have an issued created that is dedicated to the specific problem

  5. Disable current automated testing scripts. This must be done by either @hjmjohnson, @dzenanz or @thewtex as they currently have administrative control. a. CircleCI b. AppVeyor c. Travis d. Azure pipelines

  6. Delete CircleCI, AppVeyor, Travis and Azure pipeline configuration files.

  7. @hjmjohnson contacts @thewtex about Python package name, @thewtex will add @hjmjohnson to the maintainer list on PyPI.

  8. @hjmjohnson Creates an API token on PyPI for the package, and puts in the repository Settings -> Secrets under the key pypi_password.

  9. Create new Git tag, either locally and pushing, or in the GitHub web Release page, this will trigger builds and push the packages to PyPI.

  10. Update ${MODULE}.remote.cmake to reflect Grading Level Criteria Report

TODO

Below is a list of remote modules to completed for the new testing configurations. These will be completed in order from TOP to BOTTOM through the advice of @hjmjohnson.

NOTE: If a module has already been completed, please update the issue.

  • [x] Module_SphinxExamples
  • [x] Module_PerformanceBenchmarking
  • [x] Module_MGHIO
  • [x] Module_GenericLabelInterpolation
  • [x] Module_Cuberille
  • [x] Module_SimpleITKFilters
  • [x] Module_AnalyzeObjectMapIO
  • [x] Module_AnisotropicDiffusionLBR
  • [x] Module_BSplineGradient
  • [x] Module_BoneMorphometry
  • [x] Module_FixedPointInverseDisplacement
  • [x] Module_HigherOrderAccurateGrad
  • [x] Module_IOFDF
  • [x] Module_IOMeshSTL
  • [ ] Module_IOTransformDCMTK
  • [x] Module_LabelErodeDilate
  • [ ] Module_LesionSizingToolkit
  • [x] Module_MinimalPathExtraction
  • [x] Module_MorphologicalContourInt
  • [x] Module_MultipleImageIterator
  • [x] Module_ParabolicMorphology
  • [x] Module_PhaseSymmetry
  • [x] Module_PolarTransform
  • [x] Module_PrincipalComponentsAnalysis
  • [x] Module_RLEImage
  • [x] Module_RTK
  • [x] Module_SkullStrip
  • [x] Module_SplitComponents
  • [x] Module_Strain
  • [x] Module_TextureFeatures
  • [x] Module_Thickness3D
  • [x] Module_TwoProjectionRegistration
  • [ ] Module_VariationalRegistration
  • [x] Module_Montage
  • [x] Module_SCIFIO
  • [x] Module_BoneEnhancement
  • [x] Module_BioCell
  • [x] Module_MeshNoise
  • [x] Module_IsotropicWavelets
  • [x] Module_SmoothingRecursiveYvvGa
  • [x] Module_SubdivisionQuadEdgeMesh
  • [x] Module_IOScanco
  • [x] Module_AdaptiveDenoising

mseng10 avatar May 18 '20 17:05 mseng10

For modules without a setup.py and PyPI package, will there be special actions taken when implementing the new changes? If so, would an administrator be able to help direct me to these changes?

mseng10 avatar May 18 '20 19:05 mseng10

Shouldn't we disable and delete the current configuration after we make the new configuration work? So move current steps 1 and 2 to be after current step 6.

As @thewtex set up most of the current testing infrastructure, he can best advise about changes. He also has all the necessary authorizations. Me and the other admins probably have most authorizations too, but there are so many moving pieces I never learned them all 😞

dzenanz avatar May 18 '20 20:05 dzenanz

It is good to hear that remotes will have another round of updates! Thanks for working on this @mseng10.

dzenanz avatar May 18 '20 20:05 dzenanz

@dzenanz Putting steps 1 and 2 after 6, would be better practice. Thank you for catching this.

mseng10 avatar May 18 '20 20:05 mseng10

Sounds interesting @mseng10. For RTK, @LucasGandel recently setup Azure pipelines for tests and for the python packages... Before following these steps, can you confirm that they apply to modules which are not hosted by InsightSoftwareConsortium?

SimonRit avatar May 18 '20 22:05 SimonRit

For modules without a setup.py and PyPI package, will there be special actions taken when implementing the new changes? If so, would an administrator be able to help direct me to these changes?

We can remove the Python package sections from the configuration for these modules, ... or we can add Python wrapping configuration :-).

As @thewtex set up most of the current testing infrastructure, he can best advise about changes.

Most of AppVeyor has already been disabled. For CircleCI, and TravisCI, I think anyone with GitHub permissions on the repository should be able to disable them.

there are so many moving pieces I never learned them all disappointed

The recent updates will result in the removal of many moving pieces and many administrative headaches :exploding_head:

Before following these steps, can you confirm that they apply to modules which are not hosted by InsightSoftwareConsortium?

Yes, the GitHub Actions configuration to .github/workflows/build-test-package.yml -- it may work without any other modifications.

thewtex avatar May 19 '20 03:05 thewtex

@thewtex Just to clarify, we are removing Azure pipeline testing?

mseng10 avatar May 21 '20 00:05 mseng10

@mseng10 yes -- please notify @dzenanz and myself when GitHub Action are setup and we are ready to disable Azure Pipelines.

thewtex avatar May 22 '20 01:05 thewtex

Interestingly, RTK ended up being ticked but I haven't seen any change on RTK's repository. Can you explain this to me? I haven't had time to work on this myself but it's good to hear if you consider it's not necessary...

SimonRit avatar Jul 02 '20 14:07 SimonRit

@SimonRit I ticked it my mistake, the cxx and python builds still need to be integrated. The issue has been updated.

mseng10 avatar Jul 02 '20 14:07 mseng10

@thewtex Some of the remote modules require additional cmake configurations to complete their respective builds. For the cxx builds, I can simply add it to the existing configuration, but I am unsure of how to configure the python builds. Is it possible to configure additional cmake configurations when running the ITKPythonPackage scripts?

After inspecting the documentation and scripts, I was unable to find anything, but I just wanted to clarify. If not, how should we go about handling these cases?

mseng10 avatar Jul 15 '20 00:07 mseng10

@mseng10 there is not currently a way to pass additional CMake flags to the module Python packaging scripts.

What are the flags required? Is there a way to make these flags the default in the Python package builds?

thewtex avatar Jul 15 '20 15:07 thewtex

@thewtex I have only found a couple instances. An example can be module ITKIOTransformDCMTK where it needs ITKDCMTK and ITKIODCMTK. I don't believe we can set the default flags in the Python package builds, because the flags are set in various scripts in ITKPythonPackage.

mseng10 avatar Jul 21 '20 00:07 mseng10

SimonRit/RTK#376 implements it for the RTK module with some additional changes proposed in InsightSoftwareConsortium/ITKModuleTemplate#94. The build-test-package.yml file in this PR is similar to many remote modules (AdaptiveDenoising BoneEnhancement BoneMorphometry BSplineGradient Cuberille GenericLabelInterpolator HigherOrderAccurateGradient, etc.) but it seems quite different from the one in ITKModuleTemplate. Should the ITKModuleTemplate file be updated? I don't know why the pypi upload is not in ITKModuleTemplate...

SimonRit avatar Oct 22 '20 14:10 SimonRit

it seems quite different from the one in ITKModuleTemplate

Harmonizing things is welcome! But do not confuse module template's own CI with the generated project's CI.

dzenanz avatar Oct 22 '20 15:10 dzenanz

Thanks, I had indeed missed this since I modified an existing module, sorry. Except for the pypi upload which would be pointless in the template, shouldn't the two be the same?

SimonRit avatar Oct 22 '20 19:10 SimonRit

Module template also needs to invoke the cookie cutter generation step, which should not be done in regular modules. The generated project's CI should be mostly the same as regular modules (except an occasional peculiarity). PRs are welcome!

dzenanz avatar Oct 22 '20 19:10 dzenanz

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

stale[bot] avatar Jun 11 '21 01:06 stale[bot]

Has this been fully accomplished? If so, we can close the issue.

dzenanz avatar Jun 11 '21 14:06 dzenanz

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

stale[bot] avatar Apr 16 '22 11:04 stale[bot]

Part of this issue was recently handled by @tbirdso by running scripts from #3457. This might be even ripe for closing now.

dzenanz avatar Jun 02 '22 08:06 dzenanz

Delete CircleCI, AppVeyor, Travis and Azure pipeline configuration files.

There are a few remote modules that are still relying on (failing) CircleCI checks. According to the issue description these should probably be migrated to Github Actions before the issue can be closed.

tbirdso avatar Jun 02 '22 12:06 tbirdso