Remove freesurfer dependencies and add tests #29
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kdiers
force-pushed
the
remove-freesurfer
branch
2 times, most recently
from
e524818 to
2fa805c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #29 +/- ##
===========================================
+ Coverage 26.00% 63.97% +37.97%
===========================================
Files 8 8
Lines 350 322 -28
Branches 43 39 -4
===========================================
+ Hits 91 206 +115
+ Misses 256 97 -159
- Partials 3 19 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
- added new workflow 'pytest_local.yaml' to run pytest on a local runner - added environment variables for subject directory and subject id Notes: - local runner needs to be provisioned on the repository with the necessary environment variables for this workflow to work
- added new pytest to check if all required files exist in the brainprint output - removed comments from pytest_local.yaml
m-reuter
force-pushed
the
remove-freesurfer
branch
from
ad92a9a to
120a408
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims at removing the FreeSurfer dependencies from, and adding additional automated tests to the brainprint software.
For the creation of surfaces from voxel-based segmentations, we have replaced FreeSurfer's marching cube algorithm by scikit-image's marching cube algorithm. Similarly, other FreeSurfer binaries have been replaced by custom Python functions. As a result, a parallel FreeSurfer installation is no longer a requirement for running the brainprint software.
We have changed / removed the following composite structures from the brainprint shape descriptor due instable results in our evaluations of the proposed new version of the software: the left and right striatum (composite of caudate, putamen, and nucleus accumbens) and the left and right ventricles (composite of lateral, inferior lateral, 3rd ventricle, choroid plexus, and CSF) have been removed; the left and right cerebellum-white-matter and cerebellum-cortex have been merged into left and right cerebellum.
These changes are expected to BREAK COMPATIBILITY with earlier versions (0.4.0 and lower): specifically, numerical values for the brainprint shape descriptor that are obtained from the proposed new version are expected to differ from earlier versions when applied to the same data, but have been shown to remain highly correlated with earlier results in our evaluations.
Minor remaining TODOs:
pytestworkflow currently still needs to be run manually, and currently still installs the pre-release version of thelapypackage directly from its repository. This should be changed once the new lapy is released and available on pypi.pyproject.tomlmay need to be adapted to reflect the desiredlapyversion.