search

yeatmanlab / pyAFQ

Automated Fiber Quantification ... in Python
http://yeatmanlab.github.io/pyAFQ/
BSD 2-Clause "Simplified" License
53 stars 34 forks source link

[ENH] Added RUMBA-SD to data.py #1129

Closed chiuhoward closed 2 months ago

chiuhoward commented 3 months ago

Tried to add RUMBA-SD based on https://docs.dipy.org/stable/examples_built/fiber_tracking/tracking_rumba.html and https://docs.dipy.org/stable/reference/dipy.reconst.html#dipy.reconst.rumba.RumbaSDModel

pep8speaks commented 3 months ago

Hello @chiuhoward! 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! :beers:

Comment last updated at 2024-04-23 20:11:56 UTC
36000 commented 3 months ago

Maybe we could break this up into several tasks. One would be rumba_fit which returns rumba_fit. Another would be rumba_params and that can return the shm params (so this would involve running rumba_fit.odf then extract_odf). then more could be rumba_f_wm, rumba_f_gm, etc... and these functions are one-liners that accept the rumba_fit and return that tissue property. These functions would have a similar structure to dti_ga

36000 commented 2 months ago

@arokem this is ready for review/merge

chiuhoward commented 2 months ago

I think I addressed all of the changes and did the new merge correctly? =s @36000 @arokem

36000 commented 2 months ago

LGTM, Ariel?

36000 commented 2 months ago

Actually, now that I look at this, this also removed the refactoring I did so that we could use the SHM from disk from previous runs. Let me see If I can do this merge

36000 commented 2 months ago

I think this should be good to go! @chiuhoward look it over and make sure i didnt delete any changes u made recently.

chiuhoward commented 2 months ago

@36000 looks good! suffixes and docstrings were all that I changed and they’re there

36000 commented 2 months ago

OK, once the docs finish I will merge, unless @arokem has any more comments. overall, good work everyone!

arokem commented 2 months ago

Yeah - looks great! When you get a chance, maybe you can write a PR that has an example? @36000 showed me some really impressive tissue type maps from this model using the Stanford HARDI dataset. Might be nice to show slices from these as part of that.