dask / dask-ec2

Start a cluster in EC2 for dask.distributed
106 stars 37 forks source link

Fix flake8 warnings #73

Closed jbcrail closed 7 years ago

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.07%) to 73.981% when pulling c1f53c4d940ed04d117897131cb7c10a6ff80b44 on jbcrail:run-flake8 into 073a9ec924e9054b269e01bcac08766a113c3e4f on dask:master.

quasiben commented 7 years ago

@jbcrail do you think travis should fail if flake8 fails ?

quasiben commented 7 years ago

Also, Thanks!

mrocklin commented 7 years ago

Yeah, nice. A flake8 PR == <3 :)

jbcrail commented 7 years ago

@quasiben Yes, I'm partial to making Travis fail on any flake8 errors/warnings.

When resolving flake8 issues, I found an undefined method call (copy_) in commit b17ff99. The flake8 error uncovered an issue that would've arisen eventually (the method isn't invoked at this time).

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.1%) to 74.03% when pulling b77f4f9a8ca609942648e7f00abaf2328925e0ea on jbcrail:run-flake8 into e98f225ca9686f9124d0ea338babcda75748ed78 on dask:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.4%) to 74.539% when pulling 035e2597317282c462a136cbf73346389e402510 on jbcrail:run-flake8 into e98f225ca9686f9124d0ea338babcda75748ed78 on dask:master.

quasiben commented 7 years ago

@jbcrail are there other files you want to fix ? should we agree on some linting styles (column limits) and ignore versioneer.py ? I'm +1 on failing travis is linting fails, do you want to add that as well ? Happy to do all these thing but thought I'd ask since you are well into fixing these errors :)

jbcrail commented 7 years ago

@quasiben I'm finished updating the files now that flake8 passes.

I just noticed that you're using YAPF and certainly want to stick with the project's agreed-upon styling. I had originally set flake8 to use 119 (Django default), but I can update flake8 to use YAPF's defined column width of 120. I've already updated Travis to fail on flake8 errors/warnings. I also can update Travis to run flake8 only on the dask_ec2 directory.

After making these changes, should I run YAPF to format the code?

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.4%) to 74.539% when pulling e19f193e1f53d794f8afcdd78b2fd06e9eb36325 on jbcrail:run-flake8 into e98f225ca9686f9124d0ea338babcda75748ed78 on dask:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.4%) to 74.539% when pulling e19f193e1f53d794f8afcdd78b2fd06e9eb36325 on jbcrail:run-flake8 into e98f225ca9686f9124d0ea338babcda75748ed78 on dask:master.

quasiben commented 7 years ago

@jbcrail, apologies for letting this drag on. I think we should merge this and remove YAPF. Thank you so much for this contribution!

quasiben commented 7 years ago

merging