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

PEP8 in sims #884 #960

Closed
wants to merge 2 commits into from
Closed

Conversation

theaverageguy
Copy link
Contributor

Fixes #884

@@ -55,32 +55,32 @@ def test_phantom():
FA[np.isnan(FA)] = 0
# 686 -> expected FA given diffusivities of [1500, 400, 400]
l1, l2, l3 = 1500e-6, 400e-6, 400e-6
expected_fa = (np.sqrt(0.5) * np.sqrt((l1 - l2)**2 + (l2-l3)**2 + (l3-l1)**2 )/np.sqrt(l1**2 + l2**2 + l3**2))
expected_fa = np.sqrt(0.5) * np.sqrt((l1 - l2)**2 + (l2 - l3)**2 + (l3 - l1)**2) / np.sqrt(l1**2 + l2**2 + l3**2))
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is probably still >80 characters long. A suggestion:

    expected_fa = (np.sqrt(0.5) * np.sqrt((l1 - l2)**2 + 
                   (l2 - l3)**2 + (l3 - l1)**2) / 
                   np.sqrt(l1**2 + l2**2 + l3**2)))

@arokem
Copy link
Contributor

arokem commented Mar 2, 2016

Thanks for taking a look! Just a couple of small comments.

@theaverageguy
Copy link
Contributor Author

Fixing em in 5! :)

On Thu, Mar 3, 2016 at 2:16 AM, Ariel Rokem notifications@github.com
wrote:

Thanks for taking a look! Just a couple of small comments.


Reply to this email directly or view it on GitHub
#960 (comment).

@theaverageguy
Copy link
Contributor Author

Can you check now @arokem ?

# r=fvtk.ren()
# fvtk.add(r,fvtk.volume(vol234[...,0]))
# fvtk.show(r)
# vol234n=add_rician_noise(vol234,20)
Copy link
Contributor

Choose a reason for hiding this comment

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

Somebody might want to clean this up at some point, but maybe not on this PR.

@arokem
Copy link
Contributor

arokem commented Mar 3, 2016

LGTM. Someone else want to take a look here?

expected_fa = (np.sqrt(0.5) * np.sqrt((l1 - l2)**2 + (l2-l3)**2 + (l3-l1)**2 )/np.sqrt(l1**2 + l2**2 + l3**2))
expected_fa = (np.sqrt(0.5) * np.sqrt((l1 - l2)**2 +
(l2 - l3)**2 + (l3 - l1)**2) /
np.sqrt(l1**2 + l2**2 + l3**2)))
Copy link
Contributor

Choose a reason for hiding this comment

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

np.sqrt(l1**2 + l2**2 + l3**2))
remove extra brackets.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup. That's probably my bad for suggesting this. See failing tests:

https://travis-ci.org/nipy/dipy/jobs/113234014#L2306

@arokem
Copy link
Contributor

arokem commented Mar 18, 2016

Closing for now.

@arokem arokem closed this Mar 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants