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 and update all remote modules to adhere to new testing structure #1828

Open
40 of 43 tasks
mseng10 opened this issue May 18, 2020 · 23 comments
Open
40 of 43 tasks

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

mseng10 opened this issue May 18, 2020 · 23 comments
Labels
type:Design Improvement in the design of a given area

Comments

@mseng10
Copy link
Contributor

mseng10 commented May 18, 2020

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.

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

mseng10 commented May 18, 2020

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?

@dzenanz
Copy link
Member

dzenanz commented May 18, 2020

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
Copy link
Member

dzenanz commented May 18, 2020

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

@mseng10
Copy link
Contributor Author

mseng10 commented May 18, 2020

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

@SimonRit
Copy link

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?

@thewtex
Copy link
Member

thewtex commented May 19, 2020

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 🤯

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.

@mseng10
Copy link
Contributor Author

mseng10 commented May 21, 2020

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

@thewtex
Copy link
Member

thewtex commented May 22, 2020

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

@SimonRit
Copy link

SimonRit commented Jul 2, 2020

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...

@mseng10
Copy link
Contributor Author

mseng10 commented Jul 2, 2020

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

@mseng10
Copy link
Contributor Author

mseng10 commented Jul 15, 2020

@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?

@thewtex
Copy link
Member

thewtex commented Jul 15, 2020

@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?

@mseng10
Copy link
Contributor Author

mseng10 commented Jul 21, 2020

@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.

@SimonRit
Copy link

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...

@dzenanz
Copy link
Member

dzenanz commented Oct 22, 2020

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.

@SimonRit
Copy link

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?

@dzenanz
Copy link
Member

dzenanz commented Oct 22, 2020

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!

@stale
Copy link

stale bot commented Jun 11, 2021

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

@stale stale bot added the status:Backlog Postponed without a fixed deadline label Jun 11, 2021
@dzenanz
Copy link
Member

dzenanz commented Jun 11, 2021

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

@stale stale bot removed the status:Backlog Postponed without a fixed deadline label Jun 11, 2021
@stale
Copy link

stale bot commented Apr 16, 2022

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

@stale stale bot added the status:Backlog Postponed without a fixed deadline label Apr 16, 2022
@dzenanz
Copy link
Member

dzenanz commented Jun 2, 2022

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

@stale stale bot removed the status:Backlog Postponed without a fixed deadline label Jun 2, 2022
@tbirdso
Copy link
Contributor

tbirdso commented Jun 2, 2022

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.

@jhlegarreta
Copy link
Member

Slightly related to #4748: many remotes' CIs are being updated concurrently as they are visited to make the transition towards using pyproject.toml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Design Improvement in the design of a given area
Projects
None yet
Development

No branches or pull requests

6 participants