[Bioperl-l] Compliance of Bio::Seq add_SeqFeature() method

[Fields, Christopher J]

What do other FeatureHolderI do?  What does the current set of tests expect?  That would give a consensus on expected behavior, then the decision could be made.  For instance, with SeqFeature and Seq the behavior is for a single feature as the arg with a n optional expansion arg in some cases.  Based on that, I might argue that the interface itself wasn't updated when the implementations were set up.  The name of the method seems to imply a single feature is the arg (otherwise why not call it add_SeqFeatures).

Regardless, it could be fixed so that all implementations accept a list of features, which is what I think Florent means.  Dealing with the additional optional 'EXPAND' Is kind of a pain, which includes a need to check type of the last arg and DTRT, but it is possible.  

Chris

On Mar 4, 2012, at 1:28 PM, "Hilmar Lapp" <hlapp at drycafe.net> wrote:

> It does need to comply - the interface is the contract. That being said, any implementation can go *beyond* the contract in what it supports - it just can't fall short of it. So an implementation can always implement more than the contract requires, but never less or it is out of compliance.
> 
> It seems from the documentation you quote that Bio::Seq does support a single feature to be added, so that part is fine. However, there is no mention of the EXPAND option, so if that's indeed not supported (can't look at the code right now) then it is not compliant, and that should be fixed.
> 
> -hilmar
> 
> Sent with a tap.
> 
> On Mar 5, 2012, at 2:39 AM, Florent Angly <florent.angly at gmail.com> wrote:
> 
>> Hi all,
>> 
>> I have just been burned by a problem with the Bio::Seq add_SeqFeature() method. Bio::Seq is a class which implements Bio::SeqI, which itselt implements Bio::FeatureHolderI, which defines an add_SeqFeature() method as:
>>> Usage   : $feat->add_SeqFeature($subfeat);
>>>          $feat->add_SeqFeature($subfeat,'EXPAND')
>>> Function: adds a SeqFeature into the subSeqFeature array.
>>>          with no 'EXPAND' qualifer, subfeat will be tested
>>>          as to whether it lies inside the parent, and throw
>>>          an exception if not.
>>> 
>>>          If EXPAND is used, the parent''s start/end/strand will
>>>          be adjusted so that it grows to accommodate the new
>>>          subFeature
>>> Example :
>>> Returns : nothing
>>> Args    : a Bio::SeqFeatureI object
>> 
>> In comparison, the add_SeqFeature method implemented by Bio::Seq is:
>>> Title   : add_SeqFeature
>>> Usage   : $seq->add_SeqFeature($feat);
>>>          $seq->add_SeqFeature(@feat);
>>> Function: Adds the given feature object (or each of an array of feature
>>>          objects to the feature array of this
>>>          sequence. The object passed is required to implement the
>>>          Bio::SeqFeatureI interface.
>>> Returns : 1 on success
>>> Args    : A Bio::SeqFeatureI implementing object, or an array of such objects.
>> 
>> As you can see, there is a discrepancy. While the Bio::Seq method takes an array of features, Bio::FeatureHolderI states that it should take a single feature and the optional 'EXPAND' scalar.
>> 
>> It would not be very hard to modify Bio::Seq so that it complies with Bio::FeatureHolderI. One would have to make sure that the Bio::Seq feature takes the 'EXPAND' option and to have a deprecation message for any call with more than one feature.
>> 
>> First, I am missing something here or does Bio::Seq need to comply with Bio::FeatureHolderI?
>> Then, if Bio::Seq needs to be changed, I wanted to have some feedback from other wise Bioperl-ers to see if the course of action I described is adapted.
>> 
>> Best,
>> 
>> Florent
>> _______________________________________________
>> Bioperl-l mailing list
>> Bioperl-l at lists.open-bio.org
>> http://lists.open-bio.org/mailman/listinfo/bioperl-l
> 
> _______________________________________________
> Bioperl-l mailing list
> Bioperl-l at lists.open-bio.org
> http://lists.open-bio.org/mailman/listinfo/bioperl-l