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

Fix AE test error in test_peak_directions_thorough #402

Merged
merged 2 commits into from Aug 5, 2014

Conversation

Projects
None yet
3 participants
@Garyfallidis
Member

Garyfallidis commented Jul 31, 2014

This is a fix for issue
#398

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jul 31, 2014

@matthew-brett thx for pointing out. I hope this solves the problem. GGT!

@@ -219,7 +219,7 @@ def test_peak_directions_thorough():
assert_almost_equal(angular_similarity(directions, sticks), 1, 2)
AE = np.rad2deg(np.arccos(np.dot(directions[0], sticks[0])))
assert_equal(AE < 2., True)
assert_(abs(AE) < 2. or abs(AE - 180) < 2.)

This comment has been minimized.

@matthew-brett

matthew-brett Jul 31, 2014

Member

I think assert_ ends up being nose.tools.assert_true which I personally find easier to read, but no matter.

This comment has been minimized.

@MrBago

MrBago Aug 2, 2014

Contributor

Why not AE = np.rad2deg(np.arccos(abs(np.dot(directions[0], sticks[0]))))?

also you don't need the abs around AE because arccos always returns in [0, pi].

but I'm just nitpicking :) .

@@ -435,7 +435,7 @@ def test_peaksFromModel():
pam = peaks_from_model(model, data, _sphere, .5, 45, return_odf=True)
expected_shape = (len(data), len(_odf))
assert_equal(pam.odf.shape, expected_shape)
assert_true((_odf == pam.odf).all())

This comment has been minimized.

@matthew-brett

matthew-brett Jul 31, 2014

Member

This one could be assert_equal - but again - no problem.

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 5, 2014

Elef - want to merge as is, or respond to nitpicks?

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 5, 2014

I prefer not to change the nitpicks. Too desperate to finish other much more important issues. Sorry... If you both don't mind.. please go ahead and merge.

matthew-brett added a commit that referenced this pull request Aug 5, 2014

Merge pull request #402 from Garyfallidis/fix_AE_test_error
Fix AE test error in test_peak_directions_thorough

This is a fix for issue #398

@matthew-brett matthew-brett merged commit 298177c into nipy:master Aug 5, 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