FIX: PEP8 for test files

[souravsingh]

This PR fixes the files mentioned in Issues #865 #866 #867 #870 and #871.

[arokem]

I think it was easier to read before...

Did it go over 80 characters?

[arokem]

Just a couple of small comments. Otherwise, this looks good to me. But it's a lot of changes, so I would be happy if someone else could also take a look here.

[souravsingh]

I think those parts were going over 80 characters since I checked the script using pep8 and flake8.

[arokem]

I think there were some other issues there (indentation level didn't match across lines). Would you mind putting these cases back as they were? I think they were easier to read in their previous form. Thanks!

[souravsingh]

@arokem I have made the updates to the file.

[omarocegueda]

It's not necessary to break this line, this is just 68 characters long:

        domain_grid2world, np.array(codomain_shape, dtype=np.int32),

[omarocegueda]

Hi @souravsingh!,
this was a lot of work, thank you very much for taking the time to do all these fixes! I ran pep8 and flake8 on the affected files and there are only very few minor issues for this to be perfect!:

test_imwarp.py:3:1: F401 'npt' imported but unused
test_transforms.py:3:1: F401 'assert_almost_equal' imported but unused
test_transforms.py:182:9: F841 local variable 'n' is assigned to but never used
test_parzenhist.py:324:80: E501 line too long (82 > 79 characters)
test_parzenhist.py:334:80: E501 line too long (80 > 79 characters)
test_parzenhist.py:335:80: E501 line too long (81 > 79 characters)
test_parzenhist.py:337:80: E501 line too long (83 > 79 characters)
test_parzenhist.py:417:80: E501 line too long (80 > 79 characters)
test_parzenhist.py:418:80: E501 line too long (81 > 79 characters)
test_parzenhist.py:420:80: E501 line too long (83 > 79 characters)
test_parzenhist.py:473:80: E501 line too long (80 > 79 characters)
test_parzenhist.py:474:80: E501 line too long (82 > 79 characters)
test_parzenhist.py:609:80: E501 line too long (80 > 79 characters)
test_parzenhist.py:610:80: E501 line too long (80 > 79 characters)
test_parzenhist.py:611:80: E501 line too long (84 > 79 characters)
test_parzenhist.py:612:80: E501 line too long (90 > 79 characters)
test_parzenhist.py:613:80: E501 line too long (84 > 79 characters)
test_parzenhist.py:614:80: E501 line too long (85 > 79 characters)
test_parzenhist.py:615:80: E501 line too long (84 > 79 characters)
test_parzenhist.py:616:80: E501 line too long (85 > 79 characters)
test_vector_fields.py:956:47: W291 trailing whitespace
test_vector_fields.py:957:26: W291 trailing whitespace
test_vector_fields.py:958:42: W291 trailing whitespace
test_vector_fields.py:964:47: W291 trailing whitespace
test_vector_fields.py:965:26: W291 trailing whitespace
test_vector_fields.py:966:42: W291 trailing whitespace
test_vector_fields.py:1091:47: W291 trailing whitespace
test_vector_fields.py:1092:60: W291 trailing whitespace

Appart from this and my two comments above, this is ready to merge for me. Again thanks a lot for the hard work @souravsingh !

[souravsingh]

The commit is able to fix a few PEP8 coming from test_parzenhist.py.The PEP8 errors from Line 324-420 are arising from block comments and from 609-616 is coming from a for statement,which I am wondering whether it needs to be changed

[omarocegueda]

Hi @souravsingh!,
I agree with you. At the beginning I thought that following PEP8 rules everywhere even in comment blocks was kind of silly (there are many other issues that are much more important than this), but now that I have seen the tremendous effort @arokem is doing to improve the quality of our code, I realize that being as consistent as possible has a lot of value, especially for automation: it allows us to find real PEP8 issues automatically without being distracted by these unimportant issues appearing in the PEP8 report, which leaves more time for us to work on other things. So, I think it would be great if we can make pep8 and flake8's reports clean.

What about:

    # Compare the analytical and numerical (finite differences) gradient of
    # the joint distribution (i.e. derivatives of each histogram cell) w.r.t.
    # the transform parameters. Since the histograms are discrete partitions
    # of the image intensities, the finite difference approximation is
    # normally not very close to the analytical derivatives. Other sources of
    # error are the interpolation used when transforming the images and the
    # boundary intensities introduced when interpolating outside of the image
    # (i.e. some "zeros" are introduced at the boundary which affect the
    # numerical derivatives but is not taken into account by the analytical
    # derivatives). Thus, we need to relax the verification. Instead of
    # looking for the analytical and numerical gradients to be very close to
    # each other, we will verify that they approximately point in the same
    # direction by testing if the angle they form is close to zero.
    h = 1e-4

    # Make sure dictionary entries are processed in the same order regardless
    # of the platform. Otherwise any random numbers drawn within the loop
    # would make the test non-deterministic even if we fix the seed before
    # the loop. Right now, this test does not draw any samples, but we still
    # sort the entries to prevent future related failures.

and

    # Make sure dictionary entries are processed in the same order regardless
    # of the platform. Otherwise any random numbers drawn within the loop
    # would make the test non-deterministic even if we fix the seed before
    # the loop. Right now, this test does not draw any samples, but we still
    # sort the entries to prevent future related failures.

and

        C = [(invalid_vals, valid_vals, valid_points, valid_grad),
             (valid_vals, invalid_vals, valid_points, valid_grad),
             (valid_vals, valid_vals, invalid_points_dim, valid_grad),
             (valid_vals, valid_vals, invalid_points_dim, invalid_grad_dim),
             (valid_vals, valid_vals, invalid_points_len, valid_grad),
             (valid_vals, valid_vals, valid_points, invalid_grad_type),
             (valid_vals, valid_vals, valid_points, invalid_grad_dim),
             (valid_vals, valid_vals, valid_points, invalid_grad_len)]

        for s, m, p, g in C:
            assert_raises(ValueError, H.update_gradient_sparse,
                          theta, transform, s, m, p, g)

?
Again, thank you very much for your help and patience! =)

[omarocegueda]

This is much more readable! it just needs a space after the comma:

domain_grid2world, np.array(codomain_shape, dtype=np.int32),

and remove unused line 3:

 import numpy.testing as npt

This is what flake8 reports on test_imwarp.py:

test_imwarp.py:3:1: F401 'npt' imported but unused
test_imwarp.py:72:51: E231 missing whitespace after ','

[omarocegueda]

We are really close @souravsingh! just some evil trailing spaces went unnoticed!

[souravsingh]

I don't know why, But flake8 is not reporting any whitespace errors when I run it on test_parzenhist.py. I use the vim editor.

[manu-tej]

@souravsingh Use the latest version of PEP8. For downloading use 'pip' as it will get you the latest version.

[souravsingh]

@manu-tej flake8 downloads PEP8 as a dependency.I have done a pip freeze to check the list of packages.Here are the version of pep8 and flake8-
pep8==1.7.0
flake8==2.5.4

[omarocegueda]

I have the exact same version of pep8 and flake8, that's really weird!, by any chance do you have any local changes that you have not commited/pushed to the remote branch? If you look at the code as shown in github you can see the trailing spaces there. Can you see the trailing spaces in your local test_parzenhist.py file (just to make sure this is flake8 bug)?

[souravsingh]

@omarocegueda I have fixed the files.Hopefully the scripts should show no errors in flake8.

[omarocegueda]

Great! this looks good to me, I just verified that pep8 doesn't report any more issues. @arokem, this is ready to merge for me (as soon Travis finishes its tests). Thank you very much @souravsingh !

[souravsingh]

A question- I noticed a few pep8 errors in test_crosscorr and test_imaffine.Should I fix them?

[arokem]

Thanks! And thanks @omarocegueda for the review.

[souravsingh]

Thanks for merging the patch @arokem