[Bioperl-l] A better fix for Species.pm memory leak, please REVIEW.

[Chris Fields]

On Nov 23, 2008, at 6:19 AM, Sendu Bala wrote:

> Chris Fields wrote:
>> On Nov 22, 2008, at 6:21 PM, George Hartzell wrote:
>> I'm not quite sure why we are attaching a Bio::Tree::Tree to the  
>> Bio::Species, so I need to go back and review the changes from the  
>> 1.5.2 release (I'm sure there is a good reason, just can't recall  
>> and haven't had time to look).
>
> The point of the refactor was that a Species object no longer be a  
> dumb (list of) scalar, but actually 'know' what its nodes are, so  
> you can do things like compare Species and find the LCA and such.  
> You need a Bio::Tree::Tree for that.
> ...
>> I vote to go ahead and commit this (the solution seems sound), but  
>> Sendu is the one who would be best to vet as he refactored this.   
>> If there isn't word by the middle of the week you can go ahead and  
>> make the commit.
>
> I doubt I'll have time to really look at it properly in that time- 
> frame... my initial reaction is that it is a bit bizarre (we are a  
> species object intended to hold information about the species that  
> we represent, and in order to do that... we create and store another  
> species object in ourselves?!)... but if it works, it works? Is  
> there a test in Species.t that does a Species object comparison and  
> finds the LCA? Does it trigger the problem, and does it still get  
> the correct LCA after this patch?
...
> But anyway, thanks George, go ahead and commit if it all seems  
> functional.
>
> If you could summarise your solution in the bug report(s?) as well,  
> that would be great.


Agreed, though I think we need to ensure that memory leaks don't  
permeate into Bio::Taxon when we make the switch (it shouldn't, I  
think).  BTW, checking Blame/Annotate it looks like some of this stems  
from r10974:

http://code.open-bio.org/svnweb/index.cgi/bioperl/revision/?rev=10974

chris