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

ENH: GradientTable now calculates qvalues #472

Merged
merged 3 commits into from Nov 21, 2014

Conversation

Projects
None yet
4 participants
@demianw
Contributor

demianw commented Nov 21, 2014

Added an attribute to calculate the q-value table from the b-value one when big and small delta are given to the gradient table.

@demianw demianw changed the title from ENH: GradientTable now calculates to ENH: GradientTable now calculates values Nov 21, 2014

@demianw demianw changed the title from ENH: GradientTable now calculates values to ENH: GradientTable now calculates qvalues Nov 21, 2014

@maurozucchelli

This comment has been minimized.

Contributor

maurozucchelli commented Nov 21, 2014

Perfect! This is very useful also for me...

On Fri, Nov 21, 2014 at 10:14 AM, Demian Wassermann <
notifications@github.com> wrote:

Added an attribute to calculate the q-value table from the b-value one

when big and small delta are given to the gradient table.

You can merge this Pull Request by running

git pull https://github.com/demianw/dipy gradient_table_qval_calc

Or view, comment on, or merge it at:

#472
Commit Summary

  • ENH: GradientTable now calculates qvals when big and small delta are
    provided

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#472.

@@ -164,7 +164,21 @@ def test_deltas():
npt.assert_equal(bt.big_delta, 5)
npt.assert_equal(bt.small_delta, 2)
def test_qvalues():
sq2 = np.sqrt(2)/2.
bvals = 1500*np.ones(7)

This comment has been minimized.

@arokem

arokem Nov 21, 2014

Member

PEP8: spaces around operators

This comment has been minimized.

@demianw

demianw Nov 21, 2014

Contributor

Yup, but PEP8 was not enforced at all on this file so I didn't know what's the project's policy on this

This comment has been minimized.

@arokem

arokem Nov 21, 2014

Member

My policy is to comment when I see something that isn't PEP8. We're not
perfect, but we try...

On Fri, Nov 21, 2014 at 10:16 AM, Demian Wassermann <
notifications@github.com> wrote:

In dipy/core/tests/test_gradients.py:

@@ -164,7 +164,21 @@ def test_deltas():
npt.assert_equal(bt.big_delta, 5)
npt.assert_equal(bt.small_delta, 2)

+def test_qvalues():

  • sq2 = np.sqrt(2)/2.
  • bvals = 1500*np.ones(7)

Yup, but PEP8 was not enforced at all on this file so I didn't know what's
the project's policy on this


Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/pull/472/files#r20731123.

This comment has been minimized.

@matthew-brett

matthew-brett Nov 21, 2014

Member

Seems like a good PEP8 policy to me.

[0, sq2, sq2]])
qvals = np.sqrt(1500 / 6) / (2 * np.pi)
bt = gradient_table(bvals, bvecs, big_delta=8, small_delta=2)
npt.assert_equal(bt.small_delta, 2)

This comment has been minimized.

@arokem

arokem Nov 21, 2014

Member

I don't think that this test actually exercises the code you added in gradients.py. Maybe test the value of the qvals attribute?

@arokem

This comment has been minimized.

Member

arokem commented Nov 21, 2014

Thanks for adding this. The enthused endorsement by @maurozucchelli suggests that it will be immediately useful!

@arokem

This comment has been minimized.

Member

arokem commented Nov 21, 2014

Great. I'll wait for Travis to finish cleanly here, and then merge this.

arokem added a commit that referenced this pull request Nov 21, 2014

Merge pull request #472 from demianw/gradient_table_qval_calc
ENH: GradientTable now calculates qvalues

@arokem arokem merged commit 6146ac9 into nipy:master Nov 21, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment