[Bioperl-l] SeqFeature->_expand_region

[Chris Mungall]

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