Here is the PR adding this to travis on sklearn: scikit-learn/scikit-learn#7127

(and ping @Carreau, cause we talked about this for ipython)

Thanks for pinging me ! One things I'd also like to figure out is how to have it do that locally only my non-committed changes in my editor :-)

@Carreau, indeed. The noise from existing violations makes a pre-commit hook the easier solution. But we want more.

@Carreau Remind me which editor you use?

The noise from existing violations

One hopes that by the time 2.0 is out, we can get rid of most of these on master...

Great to see that there is some interest for this tool outside joblib and scikit-learn ! Full disclosure, it is not perfect. For example, one slight issue I discovered recently is that adding a new line on top of the standard two lines between two functions won't result in a failure. The reason for this is that we do a diff without a context to avoid getting failures for PEP8 violations that already exist. Without seeing the context you can not tell that the additional new line is a PEP8 violation.

It has been quite useful though because people tend to fix PEP8 violations on their own looking at the Travis status and it does free maintainers some time to focus on more important things.

flake8_diff.sh also changes from time to time and I admit to keeping it in sync completely manually between joblib and scikit-learn. Maybe we should think about a way to share it between different projects. Suggestions more than welcome!

@lesteve We have a current system for testing pep8 that is incorporated in the tests which doesn't have this particular problem (but has many others, including being extremely fragile). I wonder if we could have best of both world by combining our experience.

Something like https://flake8-diff.readthedocs.io/en/latest/index.html could be helpful? (Not sure, but it looks like a wrapper around flake8 --diff with explicit VCS support.)

There is also pep8radius, that fix pep8 on the diff. So you don't have to fix it manually.

I'm working on a bot you can mention to apply the fix and push a change on the PR. The only drawback I have for now is that the GitHub "Api" does not respect the "Allow edit from maintainers" flag, so you have to ask the user to activate the bot on their account for it to work. Which is less than optimal.

The alternative is to host a copy of the bot yourself and give it a personal access token, which is less than ideal if it is possible to get one for all python projects on GitHub which is a simple to activate as Travis.

My 2 cents from the scikit-learn side: flake8_diff.sh is very useful. Even if it has a few gotchas, it does help reviewers to focus on more important issues than flake8 problems. Also, PR authors, even the less technical ones, manage to fix flake8 errors by themselves in order to get Travis green. Maybe it does help that the Travis build error mention the line and the line numbers as they are on the PR author machine (i.e. we use the PR branch and not the result of the PR branch into master, Travis does the latter by default).

Of course I am interested by better approaches so let me know if you come up with a better solution!

From working on different projects, I agree that @lesteve is the more robust and flexible approach. The only limitation to it is the fact that it works locally and not globally. But it is robust, fast, maintainable and doesn't return failures when it shouldn't like our current system.

As expressed to @Carreau in person yesterday, I am quite concern about automatically attempts to fix pep8 compliancy automatically: I've had mixed results with those and have always resorted to fixing them by hand (even on 8k loc files). Considering it is not an automatic tool, but has to be manually activated, I am fine with testing it on different projects. (Interestingly, @Carreau's approach could automatically break our tests quite frequently :p )
@Carreau A thought about the bot: instead of pushing to the branch, could it create a PR against the original branch?

@Carreau A thought about the bot: instead of pushing to the branch, could it create a PR against the original branch?

Yes I believe it could. Shouldn't be too hard.

One other extension possibility would be for the bot to comment on/about the non-pep-8 compliant lines with a checkable markdown list and ask you to check which lines/violation you want to autofix, or ask you to mention it on specific lines when using the "review" workflow.
That's more complicated though.

I think this has been obsoleted by the switch to pytest-pep8.