UCL / PETPVC

Partial Volume Correction in PET
Apache License 2.0
50 stars 14 forks source link

Use ITK 5.2.0 for CI #73

Closed KrisThielemans closed 3 years ago

KrisThielemans commented 3 years ago

@ghisvail are you aware of any other changes needed? (e.g. do we still need the ITKReview module?)

KrisThielemans commented 3 years ago

Still the same problem #64 (brew is still using ITK 5.1 anyway)

ghisvail commented 3 years ago

To be honest, I'd rather trust my own install of third-party dependencies or official packages in Linux distributions for CI than Homebrew.

KrisThielemans commented 3 years ago

sure. but on MacOS there is no official package manager I suppose. Anyway, we know via the Travis build that it isn't really a PETPVC problem. I might flag this up on the brew webpage. Seems they put in a fix for VTK. https://github.com/Homebrew/homebrew-core/pull/60380. Maybe it's needed for ITK as well.

ghisvail commented 3 years ago

We'll see whether I hit the same issue with the conda-forge package on MacOS

ghisvail commented 3 years ago

do we still need the ITKReview module?

You are still using LabelGeometryImageFilter, so yes you do.

KrisThielemans commented 3 years ago

This all works fine, except of course the azure macos build. Phew!

@bathomas @ghisvail it could be worth keeping some CI builds with older ITK versions. Any opinion?

ghisvail commented 3 years ago

Any opinion?

I think we are still going to live with ITK v4.13 around for a while, at least in Debian / Ubuntu land. With that in mind it's probably worth to keep testing for Linux at the very least.

KrisThielemans commented 3 years ago

fair enough. So now Travis and appveyor use 5.2.0, but azure-pipelines use (as before)

ghisvail commented 3 years ago

Fyi, no problem with OSX builds on conda-forge. It builds and tests fine.

KrisThielemans commented 3 years ago

Interesting. i see that conda already uses ITK 5.2.0.. that looks promising...

bathomas commented 3 years ago

This all works fine, except of course the azure macos build. Phew!

@bathomas @ghisvail it could be worth keeping some CI builds with older ITK versions. Any opinion?

I don't have a strong opinion really. Probably best not to maintain older versions.