-
Notifications
You must be signed in to change notification settings - Fork 71
Create new script compute_simple_matrix #1045
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
Conversation
|
Hello @EmmaRenauld, Thank you for updating ! There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2024-11-28 17:26:41 UTC |
frheault
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it ! Very useful to finally have something simpler than the other one.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1045 +/- ##
=========================================
Coverage ? 69.40%
=========================================
Files ? 447
Lines ? 24045
Branches ? 3280
=========================================
Hits ? 16688
Misses ? 5961
Partials ? 1396
|
44508a9 to
4517061
Compare
arnaudbore
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments but LGTM
| start_labels = [] | ||
| end_labels = [] | ||
|
|
||
| for s in streamlines: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am 99% sure this can be done for all streamlines with map_coordinates with streamlines resamples to 2 points. I think this could be a new function 'map_endpoints'.
(sft with streamlines resamples to 2 points)
data = map_coordinates(labels, sft.streamlines._data.T, order=0)
This will give an array where all the even position are starts and all the odds are ends.
(I believe it would be a lot faster)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, I agree, but I'm not very familiar with this. Can we merge and then I let you do it? (--insert praying emoji--)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair, but add an issue to me (and @arnaudbore that can be on my hackathon list)
2c5fe03 to
addbb11
Compare
Quick description
Moved a script that I was using in dwi_ml.
To create a (streamline count) connectivity matrix directly from tractogram, without going through the whole connectivity flow process with hdf5.
Type of change
Check the relevant options.
Provide data, screenshots, command line to test (if relevant)
...
Checklist