[Bioperl-l] Recoding Bio::SimpleAlign

[Chris Fields]

On Jul 17, 2010, at 7:21 AM, Bernd Web wrote:

> Dear Jun,
> 
> Thanks for your work on this.
> Just a few remarks.
> -is changing methods names indeed needed? this will break existing
> code (for some time)

There is expected to be a standard deprecation cycle along with this.  Changes would not be merged in until after the 1.6.2 release.

> -i am not sure how naming is generally done now in BioPerl, but is
> used to be a like "eachSeq" or "each_seq", the new SimpleAlign now has
> an "_" followed by a capital. I'd stick with each_seq like names.  You
> now changed it too  "each_Seq" but gap_char for example remained
> gap_char. But surely others know better how/if to change naming.

Technically, we would like to standardize that when object(s) are returned (such as each_seq and it's previous incarnations) the method would have the class in caps, whereas simple data would not.  There have been several starts along this path, but I think various methods (next_seq in SeqIO being one) still haven't changed.

Also, just to point out, it's not terribly hard to alias these as needed:

*eachSeq = \&each_Seq;

>> 2. gap chars/missing chars are more consistent in the package
>> Default values for gap char and missing char are now set in the package.
>> Calling/Setting gap char should be made by calling $aln->gap_char("-").
> So the change here is that now the default values are actually set in
> new, instead of setting a the default by calling gap_char.

Maybe Jun could answer this, but to me this one makes sense (methods like this, which are relied upon for various reaons, need defaults for proper state).  '-' is the most commonly-used symbol.  

>> Another example, the method is renamed to give a clearer information.
>> 
>> $aln->purge is renamed into $aln->remove_redundant_Seqs
> Purge to me is a clear name in aln context ;-)

Possibly.  However alignments can hold more than sequence data (stockholm format, for example), so maybe purge() on it's own is not descriptive enough.

Jun, is remove_redundant_Seqs() an alias/reimplementation for purge()?  The name might be a little misleading.

>> $aln->splice_by_seq_pos is renamed to $aln->remove_gaps
> But there was already a remove_gaps method.
> $aln2 = $aln->remove_gaps
> Function  : Creates an aligment with gaps removed
> 
> Title   : splice_by_seq_pos
> Usage   : $status = splice_by_seq_pos(1);
> Function: splices all aligned sequences where the specified sequence
>          has gaps.
> 
> So remove_gaps now takes an optional argument to indicate a
> 'reference' sequence for splicing?
> 
> 
> Cheers,
> Bernd

This one I'm not sure about, as the two methods to me seem unrelated from name alone.  Jun?

chris