[Biopython-dev] Running pylint over Biopython

[Peter]

On Wed, Feb 24, 2010 at 3:11 AM, Eric Talevich <eric.talevich at gmail.com> wrote:
> 2010/2/23 Peter <biopython at maubp.freeserve.co.uk>
>
>> Hi all,
>>
>> Those following @Biopython on twitter or subscribed to the github RSS
>> feed for our repository will know this already, but I've been using
>> pylint today to spot some errors in Biopython.
>> http://www.logilab.org/project/pylint
>>
>> This was prompted by Eric trying this on Bio.PDB for Bug 3013 and
>> finding some issues - thank Eric, this was a valuable suggestion.
>>
>> Glad I could help. :)

Re-reading Bug 3013, we might also want to try PyChecker
as suggested by Hongbo Zhu - I've not used that before.

>> Eric, I don't have all the dependencies installed by pylint does
>> appear to dislike a few things in Bio.Phylo on the trunk:
>
> Pylint hates the way I wrote Bio.Phylo, in particular the way TreeMixin
> assumes it will be mixed with a class that has 'root' and 'is_terminal'
> attributes, and the __dict__ hack in the PhyloXML class __init__ methods --
> it can't figure out where the attributes are coming from.

Some of the "apparent false positives" I was ignoring related to
the iterator classes in Bio.SeqIO, again this seems to be valid
code which pylint can't cope with. We may want to follow up
on this (it could be a bug in pylint?).

That said, if you can think of a cleaner way to code your bits
that might be advantageous for long term maintainance. Maybe
just add a TODO comment to consider using Abstract Base
Classes once we require Python 2.6+ for Biopython (if that
looks suitable)?

> The last error was real, and I've pushed a fix to the trunk.
> Thanks for catching it.

Cool.

>> One thing this exercise has shown is that we still need
>> to do some work on the unit test coverage.
>
> Agreed. I also added a unit test for get_alignment (finally),
> and should get to TreeMixin.prune and .split soon. Then
> Bio.Phylo will have essentially 100% unit test coverage.

I didn't mean to single out just Bio.Phylo - I meant the whole
of Biopython would benefit from more unit tests. In particular,
a lot of the "minor" errors pylint helped me fix were in error
messages (e.g. wrong variable name used). This means if
a user hit the error, rather than the exception we wanted to
raise they'd get an error about our message. So, not critical,
but it suggests we need more tests to cover the exceptions
(as well as the more important tests to cover typical usage).

Peter