[Biopython-dev] pylint, was: Changes to the main repo

[Peter Cock]

2010/3/22 Peter Cock <p.j.a.cock at googlemail.com>:
> 2010/3/21 Tiago Antão <tiagoantao at gmail.com>:
>> Hi,
>>
>> I've made some changes in the main repository (my first changes with
>> github), some comments:
>> 1. Many thanks for the GitUsage wiki page. REALLY useful.
>> 2. That being said, if I did any mistakes, they are my own fault.
>> 3. I've added support for big genepop files, something I tend do be
>> asked quite a lot
>> 4. And support for haploid data (nobody really asked this)
>> 5. I remember Peter sending an email about needed corrections to the
>> code. I am afraid I've lost that email :( . If you send it to me, I
>> will do them ASAP
>> 6. New test cases and test data files
>> 7. I might add support, in the future, to Arlequin (file format and
>> application). Allowing for statistics over sequences and other goodies
>> with sequence data.
>>
>> Regards,
>> Tiago
>
> Hi Tiago,
>
> That sounds good. Regarding point 5, running pylint over the
> code reported some possible errors in Bio.PopGen. Have a
> look at this - they are all undefined variable issues:
> http://lists.open-bio.org/pipermail/biopython-dev/2010-February/007354.html
>
> I just ran it again on the latest code, and the line numbers have
> changed a tiny bit but that is all:
>
> $ pylint --disable-msg-cat=CRW --include-ids=y
> --disable-msg=E1101,E1103,E0102 -r n Bio.PopGen
> No config file found, using default configuration
> ************* Module Bio.PopGen.Async
> E0602: 78:Async.get_result: Undefined variable 'done'
> E0602: 79:Async.get_result: Undefined variable 'done'
> ************* Module Bio.PopGen.GenePop
> E0602:166:Record.split_in_pops: Undefined variable 'GenePop'
> E0602:183:Record.split_in_loci: Undefined variable 'GenePop'
> ************* Module Bio.PopGen.GenePop.Controller
> E0602: 41:_read_allele_freq_table: Undefined variable 'self'
> E0602:133:_hw_func: Undefined variable 'self'
> E0602:393:GenePopController.test_pop_hw_prob: Undefined variable 'ext'
> E0602:458:GenePopController.test_ld.ld_pop_func: Undefined variable
> 'currrent_pop'
> ************* Module Bio.PopGen.GenePop.FileParser
> E1120:219:FileRecord.remove_locus_by_name: No value passed for
> parameter 'fw' in function call
> ************* Module Bio.PopGen.SimCoal.Cache
> E0602: 79:SimCoalCache.getSimulation: Undefined variable 'Config'
> E0602: 88: Undefined variable 'Cache'
> ************* Module Bio.PopGen.SimCoal.Controller
> E0602: 47:SimCoalController.run_simcoal: Undefined variable 'Config'
>
> Peter
>

Hi Taigo,

This is looking much better after your fixes last night - just one left:

$ pylint --disable-msg-cat=CRW --include-ids=y
--disable-msg=E1101,E1103,E0102 -r n Bio.PopGen
No config file found, using default configuration
************* Module Bio.PopGen.GenePop.Controller
E0602: 41:_read_allele_freq_table: Undefined variable 'self'

Note if I turn off those particular error messages which in other
situations I had tentatively tagged as false positives, there could
be a few more issues:

$ pylint --disable-msg-cat=CRW --include-ids=y -r n Bio.PopGenNo
config file found, using default configuration
************* Module Bio.PopGen.Async
E1101: 59:Async.run_program: Instance of 'Async' has no '_run_program' member
************* Module Bio.PopGen.GenePop.Controller
E0602: 41:_read_allele_freq_table: Undefined variable 'self'
************* Module Bio.PopGen.GenePop.EasyController
E1101: 33:EasyController.get_basic_info: Module 'Bio.PopGen.GenePop'
has no 'parse' member
E1101: 43:EasyController.test_hw_pop: Instance of 'GenePopController'
has no 'test_pop_hz_prob' member
************* Module Bio.PopGen.GenePop.FileParser
E1101:197:FileRecord.remove_population: Instance of 'FileRecord' has
no 'populations' member
E1101:206:FileRecord.remove_locus_by_position: Instance of
'FileRecord' has no 'populations' member

Some of these may be harmless, for example the Async class has a
run_program method which calls _run_program, which you expect to
be implemented in any subclass. You could add a dummy method to
show the expected arguments and just raise a NotImplementedError
exception with a comment that the subclass should implement it. e.g.

    def _run_program(self, program, parameters, input_files):
        """Actually run the program, handled by a subclass (PRIVATE).

        This method should be replaced by any derived class to do
        something useful. It will be called by the run_program method.
        """
        raise NotImplementedError("This object should be subclassed")

That particular change is probably worth doing anyway from a code
clarity point of view.

Peter