Conversation
|
python3.10 is hardcoded in a few places. We should try to make it easier to upgrade to a new python version in the future, which can happen pretty often. Maybe it is even possible to replace with python3 ? |
2f413e4 to
051c9ff
Compare
|
We will also need to update the install instructions for macOS in |
85f3e65 to
d06303d
Compare
9eeec5b to
3b74988
Compare
dkuegler
left a comment
There was a problem hiding this comment.
Lots of good stuff, some minor comments.
Big things:
- Documentation of the new installation method (in doc/overview/INSTALL.md)
- Fix the workflow
- @dkuegler While the docker build seems to work, the docker "running" seems to have errors in the surface pipeline, I will have a look at that on Friday.
11920c8 to
d8b4cb1
Compare
8ce91b4 to
b45b11f
Compare
d83a22a to
c3574a3
Compare
…esurfer pruned image can use it.
f68439d to
3d493dc
Compare
3d493dc to
a8f8084
Compare
8b5891c to
ea21063
Compare
dkuegler
left a comment
There was a problem hiding this comment.
Looks good to me :)
Please change the PR from draft to ready to merge when you completed your testing.
There was a problem hiding this comment.
Pull Request Overview
This PR adds macOS build packaging tools for FastSurfer and reorganizes the project structure by moving Docker, Singularity, and build tools into a tools folder. The changes enable automated macOS package creation on release and improve project organization.
Key changes:
- New macOS packaging system with installer scripts and build tools
- Reorganized folder structure: Docker, Singularity, and build scripts moved to
tools/directory - Added FreeSurfer configuration to
pyproject.tomlfor centralized version management - Updated documentation and workflows to reflect the new folder structure
Reviewed Changes
Copilot reviewed 34 out of 40 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/read_toml.py | New utility for extracting values from TOML configuration files |
| tools/macos_build/build_release_package.sh | Main script for building macOS installer packages |
| tools/macos_build/scripts/*.sh | Post-installation and FreeSurfer linking scripts for macOS |
| tools/build/install_fs_pruned.sh | Updated FreeSurfer installation script with improved download handling and pyproject.toml integration |
| pyproject.toml | Added FreeSurfer version and URL configuration |
| doc/overview/MACOS.md | New macOS-specific installation documentation |
| doc/overview/INSTALL.md | Updated installation instructions with macOS package option |
| .github/workflows/deploy.yml | Modified to build and upload macOS packages on release |
| tools/Docker/* | Moved Docker-related files from root to tools directory with path updates |
Comments suppressed due to low confidence (2)
tools/build/install_fs_pruned.sh:68
- The embedded Python script (lines 52-68) for reading pyproject.toml is duplicated in multiple places in the codebase (also in tools/read_toml.py). Consider extracting this to a shared function or always using the dedicated tools/read_toml.py script to avoid code duplication and maintenance issues.
tools/build/install_fs_pruned.sh:22 - Typo in usage message: "install_fs_prunded" should be "install_fs_pruned.sh"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
doc build has failed due to cloudflare being down |
68dffee to
9045517
Compare
This PR contains a tool for packaging MacOS build for FastSurfer and modified workflow to automate packaging on FastSurfer release.
build_release_package.sh script packages FastSurfer along with minimised freesurfer code.
Modified deploy.yml workflow builds and uploads packaged FastSurfers to artifacts on FastSurfer release.