132 feature consider synthstrip brain extraction#150
Conversation
MarcelRosier
commented
Jul 28, 2025
- Add SynthStrip BET
- Generalize zenodo download logic
- Fix typing error
Fix weights path bug
|
/format |
|
🤖 I will now format your code with black. Check the status here. |
|
Hmm Copilot seems to be on strike today :) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
/format |
|
🤖 I will now format your code with black. Check the status here. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds SynthStrip brain extraction support as an optional dependency and refactors the Zenodo download system to be more generic. It also addresses typing issues and reorganizes the project's optional dependencies.
- Adds SynthStrip brain extraction functionality with proper model handling and conforming logic
- Generalizes Zenodo download logic from atlas-specific to a reusable ZenodoRecord class
- Updates brain extraction interface to use device parameter consistently and fixes typing annotations
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| brainles_preprocessing/brain_extraction/synthstrip.py | New SynthStrip brain extractor implementation with model loading and image processing |
| brainles_preprocessing/utils/zenodo.py | Refactored Zenodo utilities into generic ZenodoRecord class with improved error handling |
| tests/test_zenodo.py | Updated tests to work with new ZenodoRecord class structure |
| pyproject.toml | Added synthstrip optional dependency and reorganized dependencies |
| brainles_preprocessing/brain_extraction/brain_extractor.py | Updated interface to remove unused parameters and improve typing |
| brainles_preprocessing/modality.py | Updated to use device parameter for brain extraction |
| brainles_preprocessing/preprocessor/preprocessor.py | Fixed typing for use_gpu parameter and added device configuration |
| brainles_preprocessing/preprocessor/atlas_centric_preprocessor.py | Updated to use new fetch_atlases function |
| brainles_preprocessing/registration/elastix/elastix.py | Added documentation about optional dependency |
| brainles_preprocessing/registration/greedy/greedy.py | Added documentation about optional dependency |
| docs/source/brain-extraction.rst | Added documentation for new synthstrip module |
| README.md | Updated documentation to reflect new optional dependencies |
Comments suppressed due to low confidence (2)
pyproject.toml:53
- The pathlib package version 1.0.1 does not exist. The pathlib module is part of Python's standard library since Python 3.4 and should not be listed as a dependency.
nibabel = ">=3.2.1"
brainles_preprocessing/utils/zenodo.py:71
- [nitpick] Variable name 'zenodo_response' is ambiguous. Consider renaming to 'metadata_and_url' or 'zenodo_metadata' to better reflect what it contains.
zenodo_response = self._get_metadata_and_archive_url()
| device = torch.device(device) if isinstance(device, str) else device | ||
| model = self._setup_model(device=device) | ||
|
|
||
| if device == "cpu" and num_threads > 0: |
There was a problem hiding this comment.
The condition 'device == "cpu"' will always be False because device is a torch.device object, not a string. Use 'device.type == "cpu"' instead.
| if device == "cpu" and num_threads > 0: | |
| if device.type == "cpu" and num_threads > 0: |
| shape = np.array(input_nii.shape[:3]) | ||
| affine = input_nii.affine | ||
|
|
||
| assert affine is not None, "Input NIfTI image must have a valid affine." |
There was a problem hiding this comment.
Using assert for input validation in production code is not recommended. Consider raising a ValueError instead, as assertions can be disabled with -O flag.
| assert affine is not None, "Input NIfTI image must have a valid affine." | |
| if affine is None: | |
| raise ValueError("Input NIfTI image must have a valid affine.") |
There was a problem hiding this comment.
@MarcelRosier that sounds like a valid comment?
not sure about the rest.
neuronflow
left a comment
There was a problem hiding this comment.
cool stuff, please have a look at Copilot comments before merging.
Fix device check Change assert to raise ValueError
…thub.com:BrainLesion/preprocessing into 132-feature-consider-synthstrip-brain-extraction