[Biopython-dev] Further PEP8 Cleanup

[Christian Brueffer]

On 12/3/12 21:34 , Peter Cock wrote:
> On Mon, Dec 3, 2012 at 12:46 PM, Christian Brueffer
> <christian at brueffer.de> wrote:
>> Hi,
> 
> Hi Christian,
> 
> Thanks for all the pull requests sorting out issues like this, in
> terms of lines of code you'll probably be one of the top
> contributors to the next release ;)  This sort of work isn't as
> high profile as new features or bug fixes, but has a more
> subtle role in the long term of the project - making our code
> easier to follow etc. So we do appreciate these contributions.
> 
>> I just submitted pull request #102 which fixes several types of PEP8
>> warnings (found using the awesome pep8 tool).
> 
> 101 not 102? https://github.com/biopython/biopython/pull/101
> 

102 and 103 (I actually meant 103).

>> Here's what's left after those fixes:
>>
>> $ pep8 --statistics -qq repos/biopython
>> 789     E111 indentation is not a multiple of four
> 
> That's nasty - although I think we've got rid of all the tabbed
> indentation already which was also very annoying.
> 

Some code uses two spaces etc, definatelty worth fixing.

>> 673     E121 continuation line indentation is not a multiple of four
> 
> I suspect many of those are a style judgement and done that
> way to line up parentheses etc.
> 

I'll see about those and apply case by case judgement.

>> 693     E122 continuation line missing indentation or outdented
>> 171     E123 closing bracket does not match indentation of opening bracket's
>> line
>> 86      E124 closing bracket does not match visual indentation
>> 49      E125 continuation line does not distinguish itself from next logical
>> line
>> 197     E126 continuation line over-indented for hanging indent
>> 575     E127 continuation line over-indented for visual indent
>> 1092    E128 continuation line under-indented for visual indent
>> 773     E201 whitespace after '('
>> 540     E202 whitespace before ')'
>> 23543   E203 whitespace before ':'
>> 55      E211 whitespace before '('
> 
> I'd like to see E201, E202, and E211 fixed (whitespace next to
> parentheses).
> 
> The count for E203 is surprisingly high - I suspect that
> could include some large dictionaries? Note some of the
> dictionaries are auto-generated so the code to do that
> would also need fixing.
> 
>> 180     E221 multiple spaces before operator
>> 59      E222 multiple spaces after operator
>> 5848    E225 missing whitespace around operator
>> 6517    E231 missing whitespace after ','
>> 2544    E251 no spaces around keyword / parameter equals
>> 644     E261 at least two spaces before inline comment
>> 346     E262 inline comment should start with '# '
>> 156     E301 expected 1 blank line, found 0
>> 1838    E302 expected 2 blank lines, found 1
>> 364     E303 too many blank lines (2)
>> 15553   E501 line too long (82 > 79 characters)
>> 857     E502 the backslash is redundant between brackets
> 
> Fixing E502 seems a good idea, I suspect many of these are
> purely accidental due to not realising when they are redundant.
> 

Agreed.

>> 291     E701 multiple statements on one line (colon)
>> 122     E711 comparison to None should be 'if cond is None:'
>> 3707    W291 trailing whitespace
>> 1913    W293 blank line contains whitespace
>>
>> I'm not sure where to go from here with regard to what's worth fixing and
>> what would be considered repo churn (or gratuitous changes that make
>> merging of existing patches harder).
>>
>> I'd especially like to clean up E301, E302,
> 
> E301 and E302 presumable are about the recommended spacing
> between function, class and method names? If you want to fix
> them next that seems low risk in terms of complicating merges.
> 

That and spacing between functions or between a function and a new
class.

>> ... E701, E711, W291 and W293.
> 
> Did you already fix most of those in today's pull request?
> https://github.com/biopython/biopython/pull/101
> 
> If there are more cases, then by all means fix them too.
> 

I fixed some in Nexus, that was before actually using the pep8 tool.

>> Other items like E251 are more dubious, as some developers
>> seem to prefer the current style.
>>
>> What do you think?
> 
> We have a range of styles in the current code base reflecting
> different authors - and also changes in the Python conventions
> as some of the code is now over ten years old. And if any of
> my personal coding style is flagged, I'm willing to adapt ;)
> 
> (e.g. I've learnt not to put a space before if statement colons)
> 
> As you point out, the "repo churn" from fixing minor things
> like spaces around operators does have a cost in making
> merges a little harder. Things like the exception style updates
> which you've already fixed (seems I missed some) are more
> urgent for Python 3 support, so worth doing anyway.
> 

On the other hand, it's basically a one-time cost.  However I
want to fix the lowest-hanging fruit (read: the ones with the
lowest counts ;-) first.

> You've got us a lot closer to PEP8 compliance - do you think
> subject to a short white list of known cases (like module
> names) where we don't follow PEP8 we could aim to run a
> a pep8 tool automatically (e.g. as a unit test, or even a commit
> hook)? That is quite appealing as a way to spot any new code
> which breaks the style guidelines...
> 

Having a commit hook would be ideal (maybe with a possibility to
override).  This would be especially useful against the introduction of
gratuitous whitespace.  With some editors/IDEs you don't even notice it.

Chris