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 denoise #957

Merged
merged 5 commits into from Mar 18, 2016

Conversation

Projects
None yet
3 participants
@manu-tej
Contributor

manu-tej commented Mar 1, 2016

issues #875 fixed

assert_equal(sigma, np.array([0., 0., 0.]))
arr = np.zeros((3, 3, 3))
arr[0, 0, 0] = 1
sigma = estimate_sigma(arr, disable_background_masking=False, N=1)
assert_array_almost_equal(sigma, 0.10286889997472792 / np.sqrt(0.42920367320510366))
<<<<<<< HEAD

This comment has been minimized.

@arokem

arokem Mar 1, 2016

Member

These conflict markers are going to cause some trouble... You might want to run the tests on your machine before you push the code up. You can do that by running nosetests in your source-code repository (assuming you've set up you paths correctly).

This comment has been minimized.

@manu-tej

manu-tej Mar 1, 2016

Contributor

I would like to know more about running test on local machine.

This comment has been minimized.

@arokem

arokem Mar 1, 2016

Member

We use nose for testing: https://nose.readthedocs.org/en/latest/

For a gentle introduction to this approach, take a look here:
http://katyhuff.github.io/python-testing/

There are videos with a tutorial starting here:

https://www.youtube.com/watch?v=T0BE9ApIegc

On Tue, Mar 1, 2016 at 1:41 PM, Manu Tej Sharma notifications@github.com
wrote:

In dipy/denoise/tests/test_noise_estimate.py
#957 (comment):

 assert_equal(sigma, np.array([0., 0., 0.]))

 arr = np.zeros((3, 3, 3))
 arr[0, 0, 0] = 1
 sigma = estimate_sigma(arr, disable_background_masking=False, N=1)
  • assert_array_almost_equal(sigma, 0.10286889997472792 / np.sqrt(0.42920367320510366))
    +<<<<<<< HEAD

I would like to know more about running test on local machine.


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

@manu-tej manu-tej changed the title from PEP8 fixed in denoise to FIX: PEP8 n denoise Mar 2, 2016

assert_array_almost_equal(sigma, np.array([0.46291005 / np.sqrt(0.4946862482541263),
0.46291005 / np.sqrt(0.4946862482541263),
0.46291005 / np.sqrt(0.4946862482541263)]))
assert_array_almost_equal(sigma,

This comment has been minimized.

@samuelstjean

samuelstjean Mar 4, 2016

Contributor

This is hard to read, no need to break lines for the 80 characters rule everytime if it does not really help.

This comment has been minimized.

@manu-tej

manu-tej Mar 4, 2016

Contributor

@samuelstjean Should I change it to

assert_array_almost_equal(sigma,
                          np.array([0.46291005 / np.sqrt(0.4946862482541263),
                                    0.46291005 / np.sqrt(0.4946862482541263),
                                    0.46291005 / np.sqrt(0.4946862482541263)]))

or

assert_array_almost_equal(sigma, np.array([0.46291005 / np.sqrt(0.4946862482541263), 0.46291005 /  np.sqrt(0.4946862482541263), 0.46291005 / np.sqrt(0.4946862482541263)]))

@manu-tej manu-tej changed the title from FIX: PEP8 n denoise to FIX: PEP8 in denoise Mar 5, 2016

0.10286889997472792 /
np.sqrt(0.42920367320510366),
0.10286889997472792 /
np.sqrt(0.42920367320510366)]))

This comment has been minimized.

@manu-tej

manu-tej Mar 7, 2016

Contributor

@samuelstjean Should I even change here?

manu-tej added some commits Mar 7, 2016

are apparent in the denoised result.
some regions and leave others un-denoised for spatially varying noise
profiles. Consider using :func:`piesno` to estimate sigma instead if visual
inacuracies are apparent in the denoised result.

This comment has been minimized.

@arokem

arokem Mar 11, 2016

Member

Typo (was there before, but maybe we fix it here?):

"inacuracies"=> "inaccuracies"

@arokem

This comment has been minimized.

Member

arokem commented Mar 11, 2016

Looks good to me. If you want to fix that typo, please do. Otherwise, if no one objects, I can merge this in a couple of days.

@manu-tej

This comment has been minimized.

Contributor

manu-tej commented Mar 12, 2016

@arokem fixed the typo

@arokem

This comment has been minimized.

Member

arokem commented Mar 12, 2016

Thanks. This is ready to go from my point of view.

arokem added a commit that referenced this pull request Mar 18, 2016

@arokem arokem merged commit 54e56a7 into nipy:master Mar 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@arokem

This comment has been minimized.

Member

arokem commented Mar 18, 2016

closes #875

@arokem arokem referenced this pull request Mar 18, 2016

Closed

PEP8 in denoise #875

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment