Sty/pepping

[satra]

@oesteban , @miykael - this uses yapf to clean up most of the code base with respect to pep8.

i did a few things on top of michael's PR #2358 :

1. yapf -pir -e "*test_auto*" --style='{based_on_style: pep8, column_limit: 79}' nipype
2. added yapf to the spec generator to tune the auto tests.

this results in (note the pep8 checking with 99 instead of 79 - yapf doesn't fix exactly, it uses a slop factor):

$ pep8 --max-line-length=99 --statistics -qq nipype
3       E122 continuation line missing indentation or outdented
4       E124 closing bracket does not match visual indentation
2       E125 continuation line with same indent as next logical line
4       E128 continuation line under-indented for visual indent
2       E129 visually indented line with same indent as next logical line
1       E221 multiple spaces before operator
1188    E251 unexpected spaces around keyword / parameter equals
744     E302 expected 2 blank lines, found 0
83      E402 module level import not at top of file
1609    E501 line too long (106 > 99 characters)
23      E731 do not assign a lambda expression, use a def
1       W391 blank line at end of file
81      W503 line break before binary operator

[satra]

this is now concurrent with master and updates a few things that were added between when i ran things and commits to master.

this changes too many files to review closely (i think). so my suggestion would be to check out (if on average) yapf did a reasonable job (it's not perfect).

and if all our tests pass, we can merge this.

[miykael]

Thanks @satra, that makes the whole thing much easier. And yapf makes a great job it seems.

As much as I see, yapf corrects the PEP violations very well, but also does some style corrections. As an example: 98c6d11#diff-e0dc80c66ccc03d65d1d7dc071e2ba23L11.

This is in general no problem, but changes the familiar structure in some examples like this: c1900d9#diff-ed5086db91b4e4082d5c2e18e6944553L203

Only saw now that style preferences can be specified with yapf --style-help and that you already specify some in your command.

[satra]

@miykael - yes there are a few things where our default style was cleaner and the newer style is less aesthetic. but for expediency i am fine with the new style.

i'll let a few other folks comment on this before we merge.

[djarecka]

@satra - IMO, there are many lines that would look better if the limit is 99. do we have to be as strict?

[satra]

@djarecka - if we change the limit to 99 many lines will then exceed that point just given how yapf works

for our user base and developers, it's very likely that a change to 99 will not have an effect (no dot matrix printers and no vt52 terminals). do you want to give it a try and see what that results in?

[djarecka]

@satra - oh, I just thought that it's enough to change column_limit: 99 in the yapf command (but didn't understand your comments regarding the limit at the beginning of the PR).

It's not as important, I just like longer lines, but 79 is still better than 72..;-)

[djarecka]

@satra - I see what you mean with yapf. I was trying to use yapf with column_limit: 90, but have problem with creating a new PR to compare, since I'm not sure which of the changes in your PR were done automatically with yapf (or different tools) and which you made manually (and there are too many to compare...). I now you change checkspecs.py, but I'd have to have a full lists of files/commits that you changed manually. We can also leave the short line version.

[satra]

is it ok to people if we merge this?

[oesteban]

We should probably merge this while you're at the code rodeo, DYT? Given that you are all together, ready to hotfix any problems quickly. @satra?

[effigies]

Tried to resolve the conflict, but the GitHub "Resolve conflicts" thing is buggy. Will leave this to @satra to actually fix. (Feel free to force push over my merge, given that it doesn't do anything useful.)

[oesteban]

It is very buggy indeed. But if you try again exactly the same it will work out.

[effigies]

@satra Merged master to resolve conflict from #2374. Just a heads up to pull if you need to make changes.

[oesteban]

Ready?

[effigies]

👍