[Bioperl-l] SeqFeature->_expand_region

[Jason Stajich]

I'm pretty sure the subfeature expand stuff is/has-been orthogonal to
the location stuff.  In part I didn't understand it when I wrote the
location stuff in the first place.

I've raised it before someone needs to figure out what the Right Thing to
do is here.

-jason


On Wed, 2 Jul 2003, Chris Mungall wrote:

> I have some code that does this
>
> $sf->location(Bio::Location::Simple->new);
> foreach my $subsf (@sub_seq_features) {
>   $sf->add_SeqFeature($subsf, 'EXPAND');
> }
>
> If I then check the location (either on $sf->location or $sf directly) I
> see that (start, end) covers the subfeatures. however, $sf->strand == 0
>
> All the subfeatures share the same strand, so this is clearly not a
> desirable behaviour
>
> I traced this to
>
> SeqFeature::Generic->_expand_region
>
> I'm puzzled by the logic here - see the method below. it seems that when
> expanding a region, if the strand of the container is already set, the
> contained features can't change the strand of the container. I guess this
> makes some kind of sense, although I really dislike objects that try to be
> super-clever and secretly make important executive decisions on my behalf.
>
> Anyway, the problem here is that Bio::Location::Simple->new creates a
> location with undefined start and end, but strand==0. This means that
> adding subfeatures always results in a 0 strand.
>
> here is the method:
> --------------------------
> sub _expand_region {
>     my ($self, $feat) = @_;
>     if(! $feat->isa('Bio::SeqFeatureI')) {
>         $self->warn("$feat does not implement Bio::SeqFeatureI");
>     }
>     # if this doesn't have start/end set - forget it!
>     if((! defined($self->start())) && (! defined $self->end())) {
>         $self->start($feat->start());
>         $self->end($feat->end());
>         $self->strand($feat->strand) unless defined($self->strand());
>                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     } else {
>         my $range = $self->union($feat);
>         $self->start($range->start);
>         $self->end($range->end);
>         $self->strand($range->strand);
>     }
> }
> --------------------------
>
> I'd like to either remove the underlined part, or change it to
>                                     unless $self->strand
> (ie 0 strand gets overridden)
>
> perhaps an exception should be thrown if someone tries to mix strands?
>
> Or maybe the return value for Location::Atomic should be changed
>
>   # let's go ahead and force to '0' if
>   # we are requesting the strand without it
>   # having been set previously
>    return $self->{'_strand'} || 0;
>
>
> Or perhaps I'm just using the API wrongly?
>
> I have no clue what the desired behaviour here should be, but I'm happy to
> change it and document it one way or another.
>
> --
> Chris
>
> _______________________________________________
> Bioperl-l mailing list
> Bioperl-l at portal.open-bio.org
> http://portal.open-bio.org/mailman/listinfo/bioperl-l
>

--
Jason Stajich
Duke University
jason at cgt.mc.duke.edu