PEP8 violation cleanup - Nov2017

[miykael]

This PR cleans up most of the PEP8 violations mentioned in #2306, except E501 - line too long (82 > 79 characters). E501 will be attempted at a later time point, as it implies up to 10'000 mostly manual edits.

For more detailed information about the remaining violations, see #2306.

[oesteban]

May I ask to merge this after #2325?

[miykael]

I agree with @oesteban, #2355 should be merged first. Both of them have passed their checks and would be ready for merge.

[oesteban]

:D I meant #2325, so my proposal for the ordering would be #2325, #2355, #2358.

[miykael]

My PR had some conflicts with master. I corrected those conflicts with e98e51b.

For all conflicts, I chose master over my PR, except for the errors E501:

- from __future__ import print_function, division, unicode_literals, absolute_import
+ from __future__ import (print_function, division, unicode_literals,
                          absolute_import)

[oesteban]

@miykael could you please add one entry for this and for #2355 in the CHANGES file? Please remember to commit with [skip ci] in the commit message to avoid unnecessary builds.

[miykael]

@oesteban, added the entries to CHANGES. I wasn't aware about [skip ci], should I avoid this for style changes or in general not use ci too often?

[miykael]

There are still 9493 E501 violations in the code left, mostly caused by the following 29 files:

examples/dmri_connectivity.py                               (N=135)
examples/dmri_connectivity_advanced.py                      (N=132)
nipype/algorithms/tests/test_moments.py                     (N=120)
nipype/interfaces/afni/preprocess.py                        (N=107)
nipype/interfaces/ants/registration.py                      (N=288)
nipype/interfaces/ants/segmentation.py                      (N=281)
nipype/interfaces/brainsuite/brainsuite.py                  (N=351)
nipype/interfaces/camino/dti.py                             (N=248)
nipype/interfaces/cmtk/cmtk.py                              (N=129)
nipype/interfaces/freesurfer/model.py                       (N=124)
nipype/interfaces/freesurfer/preprocess.py                  (N=146)
nipype/interfaces/freesurfer/utils.py                       (N=200)
nipype/interfaces/mipav/developer.py                        (N=284)
nipype/interfaces/semtools/diffusion/diffusion.py           (N=115)
nipype/interfaces/semtools/diffusion/gtract.py              (N=274)
nipype/interfaces/semtools/registration/specialized.py      (N=109)
nipype/interfaces/semtools/segmentation/specialized.py      (N=168)
nipype/interfaces/semtools/utilities/brains.py              (N=182)
nipype/interfaces/slicer/diffusion/diffusion.py             (N=115)
nipype/interfaces/slicer/legacy/registration.py             (N=130)
nipype/interfaces/slicer/registration/specialized.py        (N=114)
nipype/workflows/dmri/camino/connectivity_mapping.py        (N=148)
nipype/workflows/dmri/fsl/epi.py                            (N=104)
nipype/workflows/dmri/mrtrix/connectivity_mapping.py        (N=180)
nipype/workflows/fmri/fsl/preprocess.py                     (N=125)
nipype/workflows/smri/ants/antsRegistrationBuildTemplate.py (N=128)
nipype/workflows/smri/freesurfer/autorecon2.py              (N=121)
nipype/workflows/smri/freesurfer/autorecon3.py              (N=251)
nipype/workflows/smri/freesurfer/recon.py                   (N=192)

Do you want me to clean them up with this PR or should I gather them in a new PR to split the review process?

Also, for the possibility that this was missed. I've opened two issues (#2356, #2357) with additonal clean up suggestions.

[oesteban]

I'd leave those E501 for another PR. BTW, is that with 79 or 99 characters long? I'd go for extending the line width a bit.

[djarecka]

+1 for extending the line width

[miykael]

This was done with max. 79 characters per line. Using 99 characters would reduce the number of errors from ~9500 to ~4000. I'm ok to change the line length to 99, but if it's about the effort to clean it. 79 length is fine for me.

The difference is anyhow not too big. The 4000 errors of 99 max length are caused by 180 files and the 9500 errors of 79 max length are caused by 225 files.

[oesteban]

Thanks a lot for this effort @miykael. It will pay back shortly after we merge it.

I'm pretty swamped right now, so I would ask @djarecka or @mgxd to have the final look and merge this PR. Thank you all for this work 👍

Regarding the line width, I see there's not a huge difference. But I presume the top 50 files more rapidly evolving in nipype will not be included in the list of 180 files, while some will in the list of 225 for 79 chars.

[djarecka]

@miykael - thanks for the work! Have you decided to move other changes to new PRs? Is this PR ready to review?

[miykael]

@oesteban and @djarecka - thanks for the nice feedback.

@djarecka - I haven't done anything more for the moment. In my opinion it's ready to be reviewed. Once it's merged, I will tackle the remaining violations (if that's ok). Perhaps best by file, so that too rapidly evolving files can be excluded from merging.

[djarecka]

@miykael since @satra made some extra changes and opened a new PR#2371, I will review the new one. But will wait to see if you have any comments.

[miykael]

@djarecka, cool. Just saw @satra's PR, makes the whole thing much simpler and easier to maintain.

As satra said, it's alot to look at. But yapf does a really good job and seems to pick up even some additional style issues that wouldn't create a PEP error, such as:

• 98c6d11#diff-06e42eed7a314186824262830403b604L10
• 98c6d11#diff-e9ad59e34e8f8b031b268e9cd3f04863L43
• 98c6d11#diff-3bc15ed0fa1a895f12e6118c7ff2c8bcR30

So for me, it's ok to review satra's PR and close this one, once everything was merged.