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

MNT: Update Ubuntu, FreeSurfer, AFNI and Convert3D #2931

Merged
merged 7 commits into from
Jan 20, 2023

Conversation

effigies
Copy link
Member

@effigies effigies commented Jan 11, 2023

Changes proposed in this pull request

  • Update Ubuntu to 22.04 LTS
  • Update FreeSurfer to 7.3.2
  • Update FSL to 6.0.6.2, download only needed packages from conda
  • Update AFNI installation to work on 22.04
  • Update ANTs to 2.4.3
  • Update Convert3D to 1.3.0, from conda-forge

@effigies effigies force-pushed the mnt/update_deps branch 2 times, most recently from b9afdb5 to 5c9acb9 Compare January 11, 2023 19:26
@effigies effigies changed the title MNT: Update Ubuntu, FreeSurfer, FSL, AFNI, ANTs and Convert3D MNT: Update Ubuntu, FreeSurfer, FSL, AFNI and Convert3D Jan 11, 2023
@effigies
Copy link
Member Author

Removing ANTs bump for now. AFAIK there are no outstanding issues we've been waiting on a new ANTs for.

@effigies effigies added this to the 23.0.0 milestone Jan 12, 2023
@effigies effigies force-pushed the mnt/update_deps branch 3 times, most recently from a4a0922 to 4d4b775 Compare January 17, 2023 02:18
@effigies
Copy link
Member Author

ICA-AROMA is the only thing stopping us from upgrading FSL: #2936.

Dockerfile Outdated Show resolved Hide resolved
@effigies effigies force-pushed the mnt/update_deps branch 2 times, most recently from 0c3e03f to 7b8dd0c Compare January 18, 2023 16:21
@effigies effigies changed the title MNT: Update Ubuntu, FreeSurfer, FSL, AFNI and Convert3D MNT: Update Ubuntu, FreeSurfer, AFNI and Convert3D Jan 19, 2023
@effigies
Copy link
Member Author

@mgxd I would appreciate a review on this one today, if possible. It would be good to do all local testing on updated dependencies.

Copy link
Collaborator

@mgxd mgxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, have you tested on any non-CI datasets? I can test with one of mine.

PS - I think having a FreeSurfer include file, rather than exclude, will lessen the burden of these migrations.

docker/files/freesurfer7.3.2-exclude.txt Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
@effigies
Copy link
Member Author

have you tested on any non-CI datasets?

I have tested on one. It will be good to do some more, but the changes relative to 7.2 were not massive.

I think having a FreeSurfer include file, rather than exclude,

The majority of the change was just running sort on the file. I think an include would be more brittle. Here, a name change means we are over-inclusive, while an include file would reverse that.

Copy link
Collaborator

@mgxd mgxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local tests didn't raise any red flags, LGTM

@effigies effigies merged commit 50a8bd9 into nipreps:master Jan 20, 2023
@effigies effigies deleted the mnt/update_deps branch August 24, 2023 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (To be released)
Development

Successfully merging this pull request may close these issues.

2 participants