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

[DOC] Update SH basis documentation #2176

Merged
merged 1 commit into from
May 29, 2020
Merged

Conversation

CHrlS98
Copy link
Contributor

@CHrlS98 CHrlS98 commented May 21, 2020

I'm doing a summer internship with @mdesco. I'm working on full spherical harmonic basis (including odd and even order terms) for reconstructing asymmetric spherical functions (coming soon in other pull request). There were inconsistencies between implementation of functions real_sym_sh_mrtrix and real_sym_sh_basis from module dipy.reconst.shm and documentation of theory on spherical harmonic bases. I fixed the doc for it to agree with the implementation.

@skoudoro skoudoro added this to the 1.2 milestone May 21, 2020
@skoudoro
Copy link
Member

Great! thank you for doing this @CHrlS98.

Let's see if there are any comments otherwise, I will merge this when Travis finishes.

@codecov
Copy link

codecov bot commented May 21, 2020

Codecov Report

Merging #2176 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2176   +/-   ##
=======================================
  Coverage   91.27%   91.28%           
=======================================
  Files         251      251           
  Lines       32421    32421           
  Branches     3420     3420           
=======================================
+ Hits        29593    29595    +2     
  Misses       2085     2085           
+ Partials      743      741    -2     
Impacted Files Coverage Δ
dipy/nn/tests/test_tf.py 88.46% <100.00%> (ø)
dipy/reconst/qtdmri.py 93.94% <100.00%> (ø)
dipy/reconst/sfm.py 92.22% <100.00%> (ø)
dipy/reconst/tests/test_sfm.py 98.77% <100.00%> (ø)
dipy/sims/voxel.py 91.09% <100.00%> (ø)
dipy/viz/app.py 77.41% <0.00%> (+0.49%) ⬆️

Copy link
Contributor

@GuillaumeTh GuillaumeTh left a comment

Choose a reason for hiding this comment

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

I recheck all the maths and that looks good to me. @skoudoro Something to do about the CI ?

@skoudoro
Copy link
Member

Thank you for your feedback @GuillaumeTh. merging

@skoudoro skoudoro merged commit 5e4adff into dipy:master May 29, 2020
@CHrlS98 CHrlS98 deleted the sh_basis_doc branch May 29, 2020 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants