ENH: Add a pep8-radius script

We've been stuck on how to handle PEP8 cleanups for a while.
On the one hand PEP8 storms break PRs and git blame, OTOH
the back and forth on code review about whitespace fixes drives some
maintainers (me) crazy.

But:

• Autopep8 accepts a line-range argument which tells it to only fix problems in a given area.
• git diff tells you which lines on which file were altered between two points in history.

So:

• write a script "pep8-radius.py" that takes a commit hash h as input, find altered lines
  between HEAD and h and then invokes autopep8 on a given radius around those
  altered lines to locally cleanup where things changed.
• Do one last pep8 storm.
• Get contributors to run the script and include/squash the cleanups in the PR.

This is a fun little hack which might become quite handy outside pandas.
would appreciate someone picking this idea up and making it happen as I'm
a little burned out on this sort of work.

[hayd]

This is a great idea. I've put something together which runs over the changed lines (I just grab the output of git diff manually) will post as another project as it's pretty generic.

I think may be some bug(s) in autopep8 when using the range argument over already indented code, it doesn't fix it. Seems to get most other infractions though.

[hayd]

@y-p A first go is here https://github.com/hayd/btyfi/blob/master/btyfi.py

pip installable as either pep8radius or Better-Than-You-Found-It... but expect bugs! (Edit: renamed)

[schodge]

I had wanted to attempt this, but didn't have time - at any rate, this
is a far nicer implementation than I would have come up with (never used
argparse before).

Would adding the check GitHub commit hash functionality just be a matter
of pulling the URL of this form:

https://stackoverflow.com/questions/22180869/get-git-commit-from-short-hash-using-github-api

using something like httplib, and then feeding into the code already
present? If so, I'll try to write it up tomorrow as a break in hunting
for a mysterious parasitic capacitance plaguing my impedance meter...

Shayne

Andy Hayden mailto:notifications@github.com
Tuesday, March 25, 2014 10:28 PM

@y-p https://github.com/y-p A first go is here
https://github.com/hayd/pep8-radius/blob/master/pep8radius.py

pip installable as pep8radius... but expect bugs!

—
Reply to this email directly or view it on GitHub
#6248 (comment).

[hayd]

@schodge I'm just naively passing rev to git diff and parse the results, I think git diff should just accept either branch name (e.g. master) or hash... But I confess to not having to fully tested (I'm not sure a good way to test these in general)!

I'm also not 100% that I'm checking the correct lines (I think what I'm actually doing is autopep8-ing those which are displayed in the git-diff, which is ok, but maybe it should be the subset of just those edited lines? Would love to have more eyes/any fixes!

[hayd]

I tagged this for 0.15, perhaps we can pep8 storm after 0.14 (as @y-p suggests) and include this script / add to dev page / wiki etc.

cc @jreback @jorisvandenbossche (not sure else may be interested)

[hayd]

I've found a couple of bugs in the line range argument of autopep8, and fixed one. Specifically indentation levels aren't fixed (e.g. tabs vs spaces - which is tricky to review in GH so would be good to fix).

(It already ensures new lines at the end of file.) Any other critical ones?

[jorisvandenbossche]

@hayd Tried it out and seems to work, very nice!
Although, I am not really sure about the name ... :-)

I think we should just more stress that new contributions have to follow pep8, and then we can point them to this to help with that. So in that case it doesn't matter if it's tagged 0.14 or 0.15, as also new contributions at this moment should ideally be pep8 compliant.

[hayd]

I think the original idea is that we'd do a big pep8 storm, then (new) contributors could be directed to this to run this before commiting / rebasing etc. hence ensuring they are PEP8-y. It's v. easy just to run it!

From above: "the back and forth on code review about whitespace fixes drives some maintainers (me) crazy." :)

[jreback]

I think we should do a pep fix, but want to work down some of the issues (your @hayd) first ....
then should do it in several commits....e.g. do a few files at a time to avoid creating massive confusion....

[hayd]

The good thing about this script, is that people can start using it before any mass pep8 fix (though I agree we should) and it won't lead to noise...

Trying to wrap my head around fix of the indentation issue... hope to have a fix soonish. I think we already have a lot (after next autopep8 release), but imo best way to test is to start using!

[jorisvandenbossche]

yes, that is what I meant that using this does not depend on whether we already did a PEP8 storm or not. For new code we can always think to ask for PEP8 compliance, and this tool can help to diminish the back and forth code review

[hayd]

@jorisvandenbossche cool. Ok will see if maybe they'll roll a point release once I fix indentation... and ping back here.

[hayd]

Fixed indentation stuff, just waiting on point release (or you can install autopep8 from github master).

Related is docstring formatting (autopep8 doesn't touch strings I don't think), not sure how well this tool would fair on pandas (would be interesting to have a play): https://github.com/myint/docformatter

...trivial to include call to this on the line ranges in pep8radius script if it worked for us.