Lots of new PEP-8 violations

[eigenhombre]

Can we have a consensus on whether or not to follow PEP-8? As @mrocklin well knows, my vote is to slavishly follow the dictates of the pep8 tool, as an aid to consistency and readability (without long discussions about style). @eriknw, do you have an opinion? Anyways, here are some outstanding errors, which I'm happy to fix if we can come to a final decision on this.

(env)09:57:08 toolz (lazy-remove) >  pep8 .
./bench/test_curry.py:5:1: E302 expected 2 blank lines, found 1
./doc/source/conf.py:6:80: E501 line too long (80 characters)
./doc/source/conf.py:14:11: E401 multiple imports on one line
./examples/graph.py:11:11: E221 multiple spaces before operator
./examples/wordcount.py:12:1: W391 blank line at end of file
./toolz/dicttoolz/tests/test_core.py:35:1: E303 too many blank lines (3)
(env)09:57:34 toolz (lazy-remove) >  

[eriknw]

Yeah, I always run the pep8 command on files I modify, and I try to follow it. Note that pep8 was revised for the first time this August: http://www.python.org/dev/peps/pep-0008/

Limiting comments and docstrings to 72 characters is something I always forget about though, and the pep8 command (at least the one I'm using) doesn't enforce this.

My vote is to follow the pep8 tool.

[eigenhombre]

In that case, if @mrocklin is willing, I suggest a separate issue to install pep8 as part of the Travis CI build.

Meanwhile I’ll work on the pep8 fixes.

On Nov 8, 2013, at 10:09 AM, Erik Welch notifications@github.com wrote:

Yeah, I always run the pep8 command on files I modify, and I try to follow it. Note that pep8 was revised for the first time this August: http://www.python.org/dev/peps/pep-0008/

Limiting comments and docstrings to 72 characters is something I always forget about though, and the pep8 command (at least the one I'm using) doesn't enforce this.

My vote is to follow the pep8 tool.

—
Reply to this email directly or view it on GitHub.

[eigenhombre]

Actually, the latest pep8 tool barfs on quite a bit more:

(toolz-env)10:51:01 toolz (lazy-remove) >  conttest pep8 .
./bench/test_curry.py:5:1: E302 expected 2 blank lines, found 1
./bench/test_curry_baseline.py:6:1: E302 expected 2 blank lines, found 1
./bench/test_first.py:6:1: E302 expected 2 blank lines, found 1
./bench/test_get.py:5:1: E302 expected 2 blank lines, found 1
./bench/test_get_list.py:6:1: E302 expected 2 blank lines, found 1
./bench/test_sliding_window.py:5:1: E302 expected 2 blank lines, found 1
./bench/test_wordcount.py:8:1: E302 expected 2 blank lines, found 1
./bench/test_wordcount.py:14:1: E302 expected 2 blank lines, found 1
./doc/source/conf.py:6:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:14:11: E401 multiple imports on one line
./doc/source/conf.py:22:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:27:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:29:80: E501 line too long (81 > 79 characters)
./doc/source/conf.py:71:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:92:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:172:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:175:1: E122 continuation line missing indentation or outdented
./doc/source/conf.py:176:1: E122 continuation line missing indentation or outdented
./doc/source/conf.py:178:1: E122 continuation line missing indentation or outdented
./doc/source/conf.py:179:1: E122 continuation line missing indentation or outdented
./doc/source/conf.py:181:1: E122 continuation line missing indentation or outdented
./doc/source/conf.py:182:1: E122 continuation line missing indentation or outdented
./doc/source/conf.py:186:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:188:3: E121 continuation line indentation is not a multiple of four
./doc/source/conf.py:213:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:226:80: E501 line too long (80 > 79 characters)
./doc/source/conf.py:232:3: E121 continuation line indentation is not a multiple of four
./doc/source/conf.py:233:80: E501 line too long (82 > 79 characters)
./doc/source/conf.py:247:80: E501 line too long (80 > 79 characters)
./examples/fib.py:5:1: E302 expected 2 blank lines, found 1
./examples/graph.py:11:11: E221 multiple spaces before operator
./examples/graph.py:23:13: E221 multiple spaces before operator
./examples/wordcount.py:3:1: E302 expected 2 blank lines, found 1
./examples/wordcount.py:12:1: W391 blank line at end of file
./toolz/dicttoolz/tests/test_core.py:11:1: E302 expected 2 blank lines, found 1
./toolz/dicttoolz/tests/test_core.py:35:1: E303 too many blank lines (3)
./toolz/functoolz/__init__.py:1:80: E501 line too long (80 > 79 characters)
./toolz/functoolz/__init__.py:2:9: E128 continuation line under-indented for visual indent
./toolz/functoolz/tests/test_core.py:2:9: E128 continuation line under-indented for visual indent
./toolz/functoolz/tests/test_core.py:130:1: E302 expected 2 blank lines, found 1
./toolz/functoolz/tests/test_core.py:133:41: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
./toolz/itertoolz/core.py:97:80: E501 line too long (82 > 79 characters)
./toolz/itertoolz/core.py:182:18: E127 continuation line over-indented for visual indent
./toolz/itertoolz/core.py:253:1: E302 expected 2 blank lines, found 1
./toolz/itertoolz/core.py:288:1: E302 expected 2 blank lines, found 1
./toolz/itertoolz/tests/test_core.py:45:80: E501 line too long (95 > 79 characters)
./toolz/itertoolz/tests/test_core.py:46:80: E501 line too long (95 > 79 characters)
./toolz/itertoolz/tests/test_core.py:47:80: E501 line too long (83 > 79 characters)
./toolz/itertoolz/tests/test_core.py:50:80: E501 line too long (91 > 79 characters)

I don't mind fixing all these, but won't until we have agreement about the overall approach.

[eriknw]

Do you want to require that the pep8 tool passses to pass the travis build test? It would be easy to do. I am supportive of generally following pep8. I would like to see the changes currently required to absolutely follow pep8. Actually, if there are rules we don't like for a file, configuring pep8 is actually really easy and really flexible.

An argument for following pep8 is, of course, having code that follows a consistent and already familiar style. This should make it easier for most people to read the code, and for new contributors to know what style to use. Plus, once pep8 is followed, it's easier to continue following it (like 100% coverage).

In other words, +1 from me, but lets see the changes before making any "big" decisions. @eigenhombre, if you clean up files, I can't see the work being for naught.

[mrocklin]

I also generally support PEP-8. There are rare cases when I don't think it's appropriate. For this reason I'm against rigidly asserting it in travis. I am for asking contributors to follow PEP-8.

I'm probably a major offender to the PEP-8 violations. I'll go through sometime this weekend and clean things up.

[mrocklin]

If you two feel strongly about this though please go ahead and add pep-8 to the travis script.

[eigenhombre]

@mrocklin @eriknw what do you think about generally following PEP-8 and rigidly enforcing some agreed-upon subset of the rules? We could add PEP-8 to Travis with any number of exceptions. I think I’m happy w/ any exceptions but strongly feel we should add something automated as a requirement to pass checkin.

On Nov 8, 2013, at 12:52 PM, Matthew Rocklin notifications@github.com wrote:

If you two feel strongly about this though please go ahead and add pep-8 to the travis script.

—
Reply to this email directly or view it on GitHub.

[eriknw]

+1, although I don't know offhand which warnings and errors we should ignore. A few other packages also use pep8 in this manner, but it doesn't appear to be common. Even though pep8 in total is viewed as a guideline, not a prescription, 90% of it is prescriptive and should be rigidly followed.

Be sure to add "--show-source" to the pep8 command to provide the best feedback to the user.

[eigenhombre]

OK, ticket is #80.

On Nov 8, 2013, at 2:26 PM, Erik Welch notifications@github.com wrote:

+1, although I don't know offhand which warnings and errors we should ignore. A few other packages also use pep8 in this manner, but it doesn't appear to be common. Even though pep8 in total is viewed as a guideline, not a prescription, 90% of it is prescriptive and should be rigidly followed.

Be sure to add "--show-source" to the pep8 command to provide the best feedback to the user.

—
Reply to this email directly or view it on GitHub.

[eigenhombre]

Matt, pls. assign to me if you no longer want this.

[mrocklin]

I'm not working on it tonight or early tomorrow but maybe tomorrow night?
Please beat me to it!
On Nov 9, 2013 5:47 PM, "John Jacobsen" notifications@github.com wrote:

Matt, pls. assign to me if you no longer want this.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/79#issuecomment-28138990
.