[Numpy-discussion] PEP8

[Nathaniel Smith]

On Mon, Sep 9, 2013 at 3:29 PM, Charles R Harris
<charlesr.harris at gmail.com> wrote:
>
>
>
> On Mon, Sep 9, 2013 at 8:12 AM, Richard Hattersley <rhattersley at gmail.com>
> wrote:
>>
>> > Something we have done in matplotlib is that we have made PEP8 a part of
>> > the tests.
>>
>> In Iris and Cartopy we've also done this and it works well. While we
>> transition we have an exclusion list (which is gradually getting shorter).
>> We've had mixed experiences with automatic reformatting, so prefer to keep
>> the human in the loop.
>>
>
> I agree with keeping a human in the loop, the script would be intended to
> get things into the right neighborhood, the submitter would have to review
> the changes after. If the script isn't too strict there will be than one way
> to do some things and those bits would rely on the good taste of the coder.

So if I understand right, the goal is to have some script that
developers can run before (or after) submitting a PR, like
  tools/autopep8-my-changes numpy/
that will fix up their changes, but leave the rest of numpy alone?

And the proposed mechanism is to come up with a combination of changes
to the numpy source and an autopep8 configuration such that
  autopep8 --our-config numpy/
becomes a no-op, and then we can use this as an implementation of
tools/autopep8-my-changes?

If that's right then my feeling is that the goal seems worthwhile but
the approach seems difficult and unlikely to survive for long. As soon
as someone overrides autopep8 once, we either have to disable the rule
for the whole project or keep overriding it manually forever. You're
already suggesting taking out the spaces-around-arithmetic rule, which
strikes me as one of the most useful -- sure, it gets things wrongs
sometimes, but I feel like we're constantly reviewing PRs where
all*the*(arithmetic+is)-written**like*this.

Maybe a better approach would be to spend that time hacking up some
script that uses git and autopep8 together to run autopep8 over all
and only those lines which the current branch has actually touched?
It's pretty easy to parse 'git diff' output to get a list of all line
numbers which have been modified, and then we could run autopep8 over
the modified files and pull out only those changes which touch those
lines.

-n

P.S.: definitely [:, :, 2]