Moved hd_bet to seperate file#179
Conversation
|
🤖 Code Formatting Reminder Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. |
1 similar comment
|
🤖 Code Formatting Reminder Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. |
|
/format |
|
🤖 I will now format your code with black. Check the status here. |
neuronflow
requested review from
IsraMekki0,
MarcelRosier and
Copilot
and removed request for
MarcelRosier and
Copilot
|
Brain extractor probably should also inherit from ABC, such as defacer. Shall we also include this fix in this PR, or make a separate PR for it? |
There was a problem hiding this comment.
Pull request overview
This PR refactors the brain-extraction implementation by moving the HDBetExtractor implementation out of brain_extractor.py into a dedicated hd_bet.py module, and updates imports/exports accordingly for consistency.
Changes:
- Move
HDBetExtractor(and itsModeenum) intobrainles_preprocessing/brain_extraction/hd_bet.py. - Update
preprocessor.pyto importHDBetExtractorfrom the new module. - Update
brainles_preprocessing.brain_extractionpackage exports in__init__.py.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| brainles_preprocessing/preprocessor/preprocessor.py | Updates imports to reference HDBetExtractor from the new module. |
| brainles_preprocessing/brain_extraction/hd_bet.py | New module containing the extracted HDBetExtractor implementation. |
| brainles_preprocessing/brain_extraction/brain_extractor.py | Removes HDBetExtractor/Mode, leaving the base extractor utilities. |
| brainles_preprocessing/brain_extraction/init.py | Adjusts package-level exports for brain extraction extractors. |
Comments suppressed due to low confidence (1)
brainles_preprocessing/brain_extraction/brain_extractor.py:13
BrainExtractordecoratesextractwith@abstractmethodbut the class does not inherit fromABC(or otherwise useABCMeta), so Python will not enforce the method as abstract. If this is intended to be an interface/base class, consider changing it toclass BrainExtractor(ABC)(and importingABC); otherwise remove@abstractmethodto avoid giving a false sense of enforcement.
class BrainExtractor:
@abstractmethod
def extract(
self,
input_image_path: Union[str, Path],
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1 +1,2 @@ | |||
| from .brain_extractor import HDBetExtractor | |||
| from .hd_bet import HDBetExtractor | |||
| from .synthstrip import SynthStripExtractor | |||
There was a problem hiding this comment.
should this be inluded?
There was a problem hiding this comment.
just saw that Synthstrip is also a fully optional and not a dependency, then yes, from my understanding this change makes sense, i will update the code
|
@nicmuenster please have a look at comments |
|
ABC inheritance is reasonable, otherwise lgtm |
neuronflow
left a comment
neuronflow
left a comment
There was a problem hiding this comment.
please include ABC inheritance (see comments above)
|
🤖 Code Formatting Reminder Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. |
|
/format |
|
🤖 I will now format your code with black. Check the status here. |
Simply moved the hd_bet class to its own seperate file for overall consistency and adapted the imports accordingly.