Skip to content
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

[REF] Refactor segmentation code into many files and pimms system #1132

Merged
merged 11 commits into from
May 31, 2024

Conversation

36000
Copy link
Collaborator

@36000 36000 commented Apr 29, 2024

This refactors the massive segmentation class into many files. It should make it a lot easier to change segmentation code in the future, ie, to make it so bundles can depend on the position of other bundles. It also makes recobundles into "just another" step in the pipeline, not as an entirely separate pipeline.

@pep8speaks
Copy link

pep8speaks commented Apr 29, 2024

Hello @36000! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-05-28 16:49:48 UTC

@36000 36000 changed the title [WIP,REF] Refactor segmentation code into many files and pimms system [REF] Refactor segmentation code into many files and pimms system Apr 29, 2024
@36000
Copy link
Collaborator Author

36000 commented Apr 30, 2024

@arokem this is ready for review/merge

AFQ/bundle_rec/recognize.py Outdated Show resolved Hide resolved
AFQ/bundle_rec/preprocess.py Outdated Show resolved Hide resolved
@36000
Copy link
Collaborator Author

36000 commented May 6, 2024

I think this is good to go, lets wait for #1124

@36000 36000 force-pushed the massive_seg_refac branch from e675c5a to 27f3183 Compare May 16, 2024 17:09
@36000
Copy link
Collaborator Author

36000 commented May 18, 2024

@arokem i added some testing and modified the criteria system to not use pimms

Copy link
Collaborator

@arokem arokem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass for now. This is a lot of code 😅

AFQ/_fixes.py Show resolved Hide resolved
AFQ/_fixes.py Show resolved Hide resolved
AFQ/bundle_rec/cleaning.py Outdated Show resolved Hide resolved
AFQ/bundle_rec/cleaning.py Outdated Show resolved Hide resolved
AFQ/bundle_rec/recognize.py Outdated Show resolved Hide resolved
AFQ/tests/test_roi.py Outdated Show resolved Hide resolved
@36000
Copy link
Collaborator Author

36000 commented May 28, 2024

@arokem this is ready for another round of review

@36000 36000 merged commit 06865e6 into yeatmanlab:master May 31, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants