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

Fix PEP8 in test_expectmax #918

Merged
merged 2 commits into from
Feb 14, 2016
Merged

Conversation

ghoshbishakh
Copy link
Contributor

fixes #863

expected[random_labels == 1, 2] = delta_field[random_labels == 1]*grad_G[random_labels == 1, 2]/sqnrm

#Pixels with gradient=0 and sigma_i_sq=0
expected[random_labels == 1, 0] = delta_field[
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of breaking the line just after the square bracket [, I think something like:

    expected[random_labels == 1, 0] = (delta_field[random_labels == 1] *
                                       grad_G[random_labels == 1, 0] / sqnrm)
    expected[random_labels == 1, 1] = (delta_field[random_labels == 1] *
                                       grad_G[random_labels == 1, 1] / sqnrm)
    expected[raendom_labels == 1, 2] = (delta_field[random_labels == 1] *
                                       grad_G[random_labels == 1, 2] / sqnrm)

would be more readable and it still fits within the 79 character limit

@omarocegueda
Copy link
Contributor

Hi @ghoshbishakh!, =)
thank you very much for your help!, I just verified that after your fix, Spyder did not report any more PEP8 issues. Apart from my comments above, this looks good to me.
Thanks!

@ghoshbishakh
Copy link
Contributor Author

thank you for reviewing. Ill try to fix the issues pointed and update PR

@arokem
Copy link
Contributor

arokem commented Feb 14, 2016

@omarocegueda - I am +1 for a merge here, if this addressed all your comments.

@omarocegueda
Copy link
Contributor

Oh! sorry I didn't notice it was done! merging now.

@omarocegueda omarocegueda merged commit 8c44f2c into dipy:master Feb 14, 2016
@omarocegueda
Copy link
Contributor

Thank you @ghoshbishakh!

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.

PEP8 in test_expectmax
3 participants