search

KitwareMedical / ITKUltrasound

ITK module with classes particularly useful for ultrasound.
http://www.insight-journal.org/browse/publication/722
Apache License 2.0
50 stars 22 forks source link

ITKUltrasound notebook checks possibly running against previous versions #177

Open tbirdso opened 2 years ago

tbirdso commented 2 years ago

In #176 notebook CI checks seem to fail with the bug that is present in the released 0.5.1 package which the PR aims to fix and tag 0.5.2. My guess is that the notebook CI is pip installing the most recent ITKUltrasound package and running notebooks with it. If this is verified to the case, we should revisit how/when we are running notebook CI.

If notebook CI does not reflect individual changes but rather tags, we should refactor so that these CI tests do not run in PRs and only run on the master branch (or maybe even only when a tagged version is released, if possible).

It seems reasonable to still run notebook CI occasionally to ensure that input data or notebook processes are not invalidated, as long as it is clearly recognized that changes since the previous tag are not reflected.

tbirdso commented 2 years ago

After bumping to 0.5.4 for factory initializations, notebook check now fails with resampleimagefilter failure

TemplateTypeError: itk.ResampleImageFilter is not wrapped for input type `itk.CurvilinearArraySpecialCoordinatesImage[itk.F,2], itk.Image[itk.F,2]`.

Related to the workaround in https://github.com/KitwareMedical/ITKUltrasound/issues/171

tbirdso commented 2 years ago

With recent fixes notebooks are now passing again 🟢

It is not practical to build Python packages every time we want to check notebooks, and whether Python packages pass/fail should not depend on whether notebooks are running correctly. Perhaps the best course of action here is to remove notebook checks from PR CI runs to make it clear that notebook checks only reflect the most recent published packages, and either leave notebook checks to run for every new commit in the master branch or even update so that notebook checks only run when a new release is tagged.

tbirdso commented 10 months ago

Note that the ITK reusable workflow build-test-package-python supports notebook testing with build artifacts. Suggest removing the dedicated test-notebook.yml file in favor of with: test-notebooks: true in that workflow.