[Bioperl-l] Bio::FeatureHolderI interface confusion

[Lincoln Stein]

On Wednesday 18 June 2003 04:22 pm, Chris Mungall wrote:
> On Wed, 18 Jun 2003, Lincoln Stein wrote:
> > To answer Chris's question, the current interface would require you to:
> >
> > 	1) open a Bio::SeqFeatureIO
> > 	2) fetch a Bio::SeqI from it
> > 	3) call Bio::SeqI->get_SeqFeatures to get one or more Bio::SeqFeatureI's
> > 	4) for each Bio::SeqFeatureI, you can rely on them to respond
> > 		to the sub_SeqFeatures() call, although they usually will
> > 		usually return an empty list
> >
> > Maybe I'm missing the point of the discussion, but there's no violation
> > of the contract here, and this is what Chris is asking for.  No need to
> > call isa() or can().
>
> Step 4 is a violation; where in the Bio::SeqFeatureI code or documentation
> does it state that you can use the method sub_SeqFeatures()?

My error.  The relevant Bio::SeqFeatureI method name is get_SeqFeatures() -- 
same as Bio::SeqI.  I think this is a bug in FeatureHolderI too, where the 
POD docs refer to sub_SeqFeatures() and the code refers to get_SeqFeatures().

Have we been arguing about a typo?

> You said: "you can rely on them to respond to the sub_SeqFeatures() call"
>
> Yes, I can rely on them to do this purely because I cheated and I ignored
> the contract and saw that the implementing class is
> Bio::SeqFeature::Generic

Oy vey.  As far as I can see, this is all the propagation of a POD typo.   
Look here at the top of the FeatureHolderI file:

  Popular feature-holders are for instance L<Bio::Seq> objects. Since
  L<Bio::SeqFeatureI> defines a sub_SeqFeature() method, most
  Bio::SeqFeatureI implementations like L<Bio::SeqFeature::Generic> will
  implement the feature holder interface as well.

This isn't true; it's called get_SeqFeatures() in Bio::SeqFeatureI.

Then below (still in FeatureHolderI):

 =head2 get_SeqFeatures

  Title   : get_SeqFeatures
  Usage   :
  Function: Get the feature objects held by this feature holder.
  Example :
  Returns : an array of Bio::SeqFeatureI implementing objects
  Args    : none

 At some day we may want to expand this method to allow for a feature
 filter to be passed in.

 =cut

 sub get_SeqFeatures{
     shift->throw_not_implemented();
 }

Clearly the right brain didn't know what the left was doing.  Then in 
Bio::Seqfeature::Generic:

  # we revamped the feature containing property to implementing
  # Bio::FeatureHolderI
  *sub_SeqFeature = \&get_SeqFeatures;
  *add_sub_SeqFeature = \&add_SeqFeature;

> Step 4 would not be a contract violation if Bio::SeqFeatureI inherited
> from Bio::FeatureHolderI - it does not.
>
> Yes, I can do a test:
>
>   if ($sf->isa("Bio::FeatureHolderI")) {
>     $sf->add_SeqFeature($foo);
>   }
>   else {
>     $self->throw("feature does not have ability to hold subfeatures");
>   }

This is yuck.  Bio::SeqI implements get_SeqFeatures().  Always has, always 
will.

sub_SeqFeature() must die!  

Lincoln

-- 
========================================================================
Lincoln D. Stein                           Cold Spring Harbor Laboratory
lstein at cshl.org			                  Cold Spring Harbor, NY
========================================================================