make pep8 test routine reusable for other projects

[megies]

I'd like to reuse the pep8 testing routine from matplotlib in one of our projects (since it's a dependency anyway) rather than duplicate code (and maintenance) by copy/patching it into our codebase. These are the minimal changes necessary for reusability.

Nothing changes for matplotlib test case but the routine can now be imported and used from another project.
Also, it's better readable I think, to have all the custom exclusions grouped at the head of the file.
I also had to move the currently active manual skip to the bottom.

Furthermore, in principle I think the whole logic rather belongs into /testing/.. and should only be imported and called in /tests/.., but I did not want to change this without a go-ahead (and its not that important, I guess).

Also, I will include a check if failing files are untracked to exclude them from the results by using a system call to git (try/excepted if git is available of course). If this PR gets a nod, I would offer to implement this right here, otherwise I'll do that in our test setup.

[pelson]

In principle this is absolutely fine. The big problem is that the way this is architected, means that nose will pick up this test even though you've skipped it in main.

I think the only solution to this currently is to rename the test function to something like assert_pep8, and then add a commented out function called "test_pep8" (or some other nose compatible name) which calls it but is currently commented out. Your code would then call "assert_pep8" rather than the test function itself.

How does that sound?

[megies]

Ok, we don't use nose so I didn't know about that aspect.

Hmm.. I think I got a knot in my brain reading your recommendation.

How about the following?

• move the logic to /testing/code_standards.py as I mentioned above
• have a slim /tests/test_code_standards.py that imports the logic, has the custom file exception rules and runs the test. We could put a raise skipTest in there without problems.

EDIT: Oh ok, see what you mean now. Actually its the same as what I was thinking just without moving the logic to /testing. So you prefer to leave everything in this file here? I'll go with your suggestion then.

[WeatherGod]

With nose, you can decorate a function as nottest:

http://nose.readthedocs.org/en/latest/testing_tools.html#nose.tools.nottest

[megies]

Actually, after I implemented to reuse this, another team member of our project wants to go for flake8 instead (to have even more warnings... ;)). So, we will probably not reuse the pep8 checker routine from here, after all.

So, no need to merge this from my point of view, after all (although I still think this improves readabillity).
I'm very sorry to have stolen your time, guys..! :(

If you still want to merge this I can use the decorator for the skipping as suggested above..

[pelson]

I think some good work has gone into this - I'm not convinced that this is the final step to allow people to re-use the PEP8 test of matplotlib, but it is a start. As such, I'm happy enough to merge this now.

Thanks @megies.

[megies]

Actually one really can reuse it, I tried and it worked ok (see PR in our project and the failed test case it produced). I would still move the logic (the StandardReportWithExclusions class and the (now renamed) assert_pep8_conformance function) over to /testing/ where people can find it, if they search for the testing logic (we're happily reusing the image comparison function by the way, thanks for that :)).

Thanks for merging.

Also, I implemented to automatically except untracked files from the checking in our project (see commit). I would offer to add this in matplotlib too via a PR if you are interested to have it.