Reformat code and fix some pip8 issues.

[aristippe]

A pain to compare but hopefully it's ok and looks not bad. I think there's at least one other double # in a comment, I can change. If there's more. I'll leave them for some other time.

[aristippe]

I vaguely recall in some cases space before and after operators like equals, and another case not. If there are any introduced changes that are better reverted, let me know.

[sinergatis]

Thanks for the effort! I'm a bit worried about the long lines, which are more than I initially noticed and they are making things rather hard to read in some places (specifically, urls.py and similar cases where it seems your tool basically indents them under the last character of the previous line). That is actually one PEP8 rule that I'd personally like to follow as much as possible (with the occasional exceptions, such as long URLs or cases where breaking the line off is cumbersome, of course).

Would it be OK if I take care of the long lines?

Also, seeing that fabfile.py has been excluded for the change, perhaps deploy/* should be excluded as well (or the opposite, include fabfile.py), just for consistency? The changes should be harmless in either case.

[aristippe]

Thanks a lot for taking a look. It is a fair amount of changes though hopefully that's it for large ones, and later ones are easier to track. Certainly change whatever you think is needed. In urls.py, yes it's auto-indenting according to E128 continuation line under-indented for visual indent. If things look good, let me know what is the next step, merge and then you make whatever changes and I merge them back?

[sinergatis]

In urls.py, yes it's auto-indenting according to E128 continuation line under-indented for visual indent.

No worries, that was indeed a bit tricky to spot as it was actually caused by the '' on the first line of the patterns definition:

urlpatterns = patterns('',
                       ^^
...

If things look good, let me know what is the next step, merge and then you make whatever changes and I merge them back?

Actually the magic of pull requests based on a branch (plus me being a collaborator on your repo :bowtie: ) should allow me to just commit to aristippe/pip8-reformat and get the pull request updated! A practical demonstration coming hopefully in a few minutes!

[sinergatis]

Following up on the github "magic", for future reference: if you have an open pull request based on a branch on your repository, any commits you push to that branch will automatically be appended to that pull request. It's quite handy for collaborating!

I have made some changes that basically fix the long-lines problem and take care of other PEP8 and flake8 warnings - hopefully all of them. An usually decent trick when you have a long string is to take advantage of the fact that ('hello' 'foo' 'bar') is equivalent to 'hellofoobar' in Python, and the first form can be broken into several lines gracefully.

Feel free to make a decision on the fabfile.py and deploy/* issue, and to merge if you are happy with the result (or otherwise suggest other changes)!

[aristippe]

Cool. Makes it easy!

Pushed a few more changes. About the indents in books/atom.py, not sure if there is some convention to align with 'not'. The change in epub.py:203 seems to be a common (preferred?) indentation[1] as well as consistant with other cases in the project. I found two more afterwards and added another commit. If that is good, then I'll merge.

Thanks again for taking a look! Such a tedious thing formatting. Hopefully that's it.

[1] https://stackoverflow.com/questions/3985563/python-best-practice-for-lists-dictionary-etc

[sinergatis]

Nice! I did however made a hopefully final round of fixes, as some of the changes introduced on your latest commits did actually break PEP8 compatibility (and fabfile.py was complaining about some stuff when running pep8 - I left some flake8 warnings there about unused variables and undefined names as they seem to be actually valid and a result of running the fabric deployment tool, which I'm not that familiar with, so I leaned on the safe side).

Perhaps we can agree on a "soft and not strictly enforced" convention like if there are several valid ways of formatting some stuff, try to pick one that does not cause PEP8 errors? It is basically the guideline I followed on this last commit:

About the indents in books/atom.py, not sure if there is some convention to align with 'not'.

Usually keeping all the clauses with the same indentation (ie. line break after the and or or, regardless if they are negated or not) does the trick - in the case of L509 and L517, the indentation was a bit confusion and under-indented according to PEP. Those ifs are a bit convoluted anyway, it was an edge case.

The change in epub.py:203 seems to be a common (preferred?) indentation[1] as well as consistant with other cases in the project. I found two more afterwards and added another commit.

Sure, whatever works for you is fine - I tend to alternate between that one but probably use the one mentioned on the second reply (by danlei) more, specially in the cases where it provides more room for long lines. That is basically the reason for the change in views.py (the allow_public_add_book was throwing a long line error).

I'm hoping this finally close the issue - with the latest commit, all files are PEP8 and flake8 compatible except for the aforementioned fabfile.py! :green_heart:

[aristippe]

No prob. Thanks again for looking through it! I understand such things are not so fun, especially after a few rounds. I'll close the issue. I'll be glad to move on too. :)

Pycharm does like to reintroduce the changes undone in books/atom.py and fabfile.py. Those indeed as you say break PEP8 and I'll file a bug. I'll look out for it for sure and try to avoid future reformats except for changed lines.

The warnings in fabfile.py go away as soon as fabric is added to requirements.txt. Final change since someday at least I might try it out just to be curious about how fabric works.