impactlab / eemeter

‼️ MOVED TO https://github.com/openeemeter/eemeter - Core computation engine for the Open Energy Efficiency Meter
https://eemeter.readthedocs.io/
MIT License
25 stars 13 forks source link

Add flake8 #133

Closed crolfe closed 8 years ago

crolfe commented 8 years ago

This change adds pytest-flake8 as a dev dependency, which integrates with pytest and checks the code for pep8 issues every time the tests are run. There are two config files that have been added:

In addition to adding this plugin, I have gone through the repo and fixed all of the issues reported by flake8, which makes up the bulk of this pull request. Note: I used autopep8 to fix a lot of the whitespace issues, which in some cases resulted in some funky formatting. I tried to fix these as I came across them, but there could be a few more still lurking, so let me know if there's anything that looks odd.

Another thing I did that doesn't appear to be caught by flake8 is import ordering. I can't remember if it is a PEP8 thing or just a convention, but best practice for import ordering usually looks something like this:

import <standard library package> from <standard library> import <module>

import <third party library> from <third party library> import <module>

import <project package> from <project> import <module>

philngo commented 8 years ago

Nice! This is a greatly helpful set of changes. A couple of thoughts: 1) It appears that the tests are broken on travis, so we should get those working before we can merge this in. Appears to have something to do with test fixtures. 2) I noticed that you did a bit of refactoring with large fixture and yaml files. Those are good changes, but see my line note. 3) autopep8 did some funky stuff on a couple of lines, pointed out in a line note. 4) All that ^^ said, the v0.4 release in the next couple of weeks will have enough breaking and merge-conflict type changes that I'm reluctant to have you spend much more time on this in the immediate future, for fear that the vast majority of work will be made somewhat irrelevant by the big upcoming changes. So out of respect for your valuable time, let's hold off on further changes and the above fixes until things on that front stabilize a bit.