-
Notifications
You must be signed in to change notification settings - Fork 2
Update freesurfer #4
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
base: main
Are you sure you want to change the base?
Conversation
GitHK
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks OK, just remove the todo's if you are done with them. Don't keep them in the code, open an issue
| <!-- TODO'S | ||
| - DONE fix build process | ||
| - bump version to 2.2.0 after bump in FreeSurfer + make sure pipeline Fariba runs end to end | ||
| - add version details in the README | ||
| - add badges | ||
| - then keep going through modernization plan (reduce image size, build time etc); add these learning to my Obsidian notes | ||
| - in future, download FreeSurfer 8.1.0 .deb and put in filesrv and fetch from there (like in ASCENT service) so we can have the latest version without increasing build time | ||
| --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address this TODO and remove it
|
Im not done with them, that is why I kept them. But was prioritizing making the CI green, will tackle those in a subsequent PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the FreeSurfer installation to version 7.4.1 and modernizes the Docker build process by implementing a multi-stage build that copies FreeSurfer from its official Docker image instead of downloading from slow FTP servers. Additionally, it adds version pinning for Python dependencies and improves build efficiency with a new .dockerignore file.
Key Changes:
- Upgraded FreeSurfer from version 6.0.0 to 7.4.1 using multi-stage Docker build
- Pinned all Python package versions in
requirements.inwith release dates for reproducibility - Added comprehensive
.dockerignorefile to optimize Docker build context
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Dockerfile | Implements multi-stage build to copy FreeSurfer 7.4.1 from official image; updates Qt5 dependencies; improves wxPython installation process |
| requirements.in | Adds version pins for all packages (nibabel, pyvista, PyOpenGL, fsleyes, etc.) with release date comments |
| README.md | Documents FreeSurfer version 7.4.1; adds TODO comments for future work; comments out Spinal Cord Toolbox reference |
| .dockerignore | New file excluding Git files, documentation, test data, and build artifacts from Docker context |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
…/jupyter-medimproc into update-freesurfer
pcrespov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why am i owner of a repo I never saw before :-)
At first sight, it LGTM
thx
No description provided.