From bee31908dc806b0dbb31f74d69b5b439e85df6c9 Mon Sep 17 00:00:00 2001
From: Arne Stabenau <stabenau@sanger.ac.uk>
Date: Mon, 1 Mar 2004 15:58:28 +0000
Subject: [PATCH] small changes in API related to Attributes

---
 .../Bio/EnsEMBL/DBSQL/MiscFeatureAdaptor.pm   |  60 +++++--
 modules/Bio/EnsEMBL/MiscFeature.pm            | 155 +++++++++++-------
 modules/Bio/EnsEMBL/Slice.pm                  |  59 +++----
 modules/t/miscFeature.t                       |  56 ++++---
 modules/t/miscFeatureAdaptor.t                |  12 +-
 5 files changed, 201 insertions(+), 141 deletions(-)

diff --git a/modules/Bio/EnsEMBL/DBSQL/MiscFeatureAdaptor.pm b/modules/Bio/EnsEMBL/DBSQL/MiscFeatureAdaptor.pm
index f86bc9f50f..1055ca3fd2 100644
--- a/modules/Bio/EnsEMBL/DBSQL/MiscFeatureAdaptor.pm
+++ b/modules/Bio/EnsEMBL/DBSQL/MiscFeatureAdaptor.pm
@@ -49,6 +49,7 @@ package Bio::EnsEMBL::DBSQL::MiscFeatureAdaptor;
 use strict;
 use Bio::EnsEMBL::DBSQL::BaseFeatureAdaptor;
 use Bio::EnsEMBL::MiscFeature;
+use Bio::EnsEMBL::Attribute;
 use Bio::EnsEMBL::MiscSet;
 use Bio::EnsEMBL::Utils::Exception qw(throw warning);
 
@@ -214,8 +215,10 @@ sub _columns {
 	     mf.seq_region_end
 	     mf.seq_region_strand
 	     ma.value
-       at.code
-       mfms.misc_set_id);
+	     at.code
+	     mfms.misc_set_id
+	     at.name
+	     at.description);
 }
 
 
@@ -284,11 +287,13 @@ sub _objs_from_sth {
   my %sr_cs_hash;
 
   my($misc_feature_id, $seq_region_id, $seq_region_start, $seq_region_end,
-     $seq_region_strand, $attrib_value, $attrib_type_code, $misc_set_id);
+     $seq_region_strand, $attrib_value, $attrib_type_code, $misc_set_id,
+     $attrib_type_name, $attrib_type_description );
 
   $sth->bind_columns( \$misc_feature_id, \$seq_region_id, \$seq_region_start,
                       \$seq_region_end, \$seq_region_strand,
-                      \$attrib_value, \$attrib_type_code,\$misc_set_id);
+                      \$attrib_value, \$attrib_type_code,\$misc_set_id,
+		      \$attrib_type_name, \$attrib_type_description );
 
   my $asm_cs;
   my $cmp_cs;
@@ -334,15 +339,25 @@ sub _objs_from_sth {
       if($misc_set_id) {
         my $misc_set = $ms_hash{$misc_set_id} ||=
           $msa->fetch_by_dbID($misc_set_id);
-        #doesn't matter if it is added twice in same slot
-        $feat_misc_sets->{$misc_set->{'code'}} = $misc_set;
+        if( ! exists $feat_misc_sets->{$misc_set->{'code'}} ) {
+	  $feat->add_MiscSet( $misc_set );
+	  $feat_misc_sets->{$misc_set->{'code'}} = $misc_set;
+	}
       }
 
       #if there is a new attribute add it to the current feature
       if($attrib_value && $attrib_type_code &&
          !$seen_attribs->{"$attrib_type_code:$attrib_value"}) {
-        $feat_attribs->{$attrib_type_code} ||= [];
-        push @{$feat_attribs->{$attrib_type_code}}, $attrib_value;
+	my $attrib = Bio::EnsEMBL::Attribute->new
+	  ( -CODE => $attrib_type_code,
+	    -NAME => $attrib_type_name,
+	    -DESC => $attrib_type_description,
+	    -VALUE => $attrib_value
+	  );
+	
+	
+        $feat_attribs ||= [];
+        push @$feat_attribs, $attrib;
         $seen_attribs->{"$attrib_type_code:$attrib_value"} = 1;
       }
 
@@ -419,15 +434,15 @@ sub _objs_from_sth {
         $slice = $dest_slice;
       }
 
-      if($misc_set_id) {
-        #get the misc_set object
-        my $misc_set = $ms_hash{$misc_set_id} ||=
-          $msa->fetch_by_dbID($misc_set_id);
-        $feat_misc_sets->{$misc_set->{'code'}} = $misc_set;
-      }
 
       if($attrib_value && $attrib_type_code) {
-        $feat_attribs->{$attrib_type_code} = [$attrib_value];
+	my $attrib = Bio::EnsEMBL::Attribute->new
+	  ( -CODE => $attrib_type_code,
+	    -NAME => $attrib_type_name,
+	    -DESC => $attrib_type_description,
+	    -VALUE => $attrib_value
+	  );
+        $feat_attribs = [$attrib];
         $seen_attribs->{"$attrib_type_code:$attrib_value"} = 1;
       }
 
@@ -438,9 +453,18 @@ sub _objs_from_sth {
           'slice'   => $slice,
           'adaptor' => $self,
           'dbID'    => $misc_feature_id,
-          'attributes' => $feat_attribs,
-          'sets'    => $feat_misc_sets});
+          'attributes' => $feat_attribs });
       push @features, $feat;
+
+      if($misc_set_id) {
+        #get the misc_set object
+        my $misc_set = $ms_hash{$misc_set_id} ||=
+          $msa->fetch_by_dbID($misc_set_id);
+        if( ! exists $feat_misc_sets->{$misc_set->{'code'}} ) {
+	  $feat->add_MiscSet( $misc_set );
+	  $feat_misc_sets->{$misc_set->{'code'}} = $misc_set;
+	}
+      }
     }
   }
 
@@ -466,6 +490,8 @@ sub list_dbIDs {
    return $self->_list_dbIDs("misc_feature");
 }
 
+
+
 1;
 
 
diff --git a/modules/Bio/EnsEMBL/MiscFeature.pm b/modules/Bio/EnsEMBL/MiscFeature.pm
index 38008c0cc6..4461fa0dc6 100644
--- a/modules/Bio/EnsEMBL/MiscFeature.pm
+++ b/modules/Bio/EnsEMBL/MiscFeature.pm
@@ -110,12 +110,29 @@ sub new_fast {
 =cut
 
 sub add_attribute {
-  my ($self, $type, $value) = @_;
+  throw( "You need to make Attribute objects now, use add_Attribute" );
+}
+
+=head2 add_Attribute
+
+  Arg [1]    : Bio::EnsEMBL::Attribute $attribute
+  Example    : $misc_feature->add_attribute($attribute);
+  Description: Adds an attribute to this misc. feature
+  Returntype : none
+  Exceptions : throw on wrong argument type
+  Caller     : general
 
-  throw('Type argument is required') if(!$type);
+=cut
+
+sub add_Attribute {
+  my ($self, $attrib) = @_;
+
+  if( ! defined $attrib || ! $attrib->isa( "Bio::EnsEMBL::Attribute" )) {
+    throw( "You have to provide a Bio::EnsEMBL::Attribute, not a [$attrib]" );
+  }
 
-  $self->{'attributes'}->{$type} ||= [];
-  push @{$self->{'attributes'}->{$type}}, $value
+  $self->{'attributes'} ||= [];
+  push @{$self->{'attributes'}}, $attrib
 }
 
 
@@ -133,61 +150,42 @@ sub add_attribute {
 =cut
 
 sub add_set {
-  my ($self, $set) = @_;
-
-  if(!$set || !ref($set) || !$set->isa('Bio::EnsEMBL::MiscSet')) {
-    throw('Set argument must be a Bio::EnsEMBL::MiscSet');
-  }
-
-  my $code = $set->code();
-
-  throw('Set must be associated with a code to be added') if(!$code);
-
-  $self->{'sets'}->{$code} = $set;
+  throw( "Use add_MiscSet instead." );
 }
 
 
+=head2 add_MiscSet
 
-=head2 get_attribute_types
-
-  Arg [1]    : none
-  Example    : @attrib_types = $misc_feature->get_attribute_types();
-  Description: returns a list of attribute types that this feature has
-  Returntype : list of strings
-  Exceptions : none
+  Arg [1]    : Bio::EnsEMBL::MiscSet $set
+               The set to add
+  Example    : $misc_feature->add_MiscSet(Bio::EnsEMBL::MiscSet->new(...));
+  Description: Associates this MiscFeature with a given Set.
+  Returntype : none
+  Exceptions : throw if the set arg is not provided,
+               throw if the set to be added does not have a code
   Caller     : general
 
 =cut
 
-sub get_attribute_types {
-  my $self = shift;
-  $self->{'attributes'} ||= {};
-
-  return keys %{$self->{'attributes'}};
-}
 
-=head2 get_set_codes
+sub add_MiscSet {
+  my $self = shift;
+  my $miscSet = shift;
 
-  Arg [1]    : none
-  Example    : @set_codes = $misc_feature->get_set_codes();
-  Description: returns a list of codes for the sets this feature is associated
-               with
-  Returntype : list of strings
-  Exceptions : none
-  Caller     : general
+  if(!$miscSet || !ref($miscSet) || !$miscSet->isa('Bio::EnsEMBL::MiscSet')) {
+    throw('Set argument must be a Bio::EnsEMBL::MiscSet');
+  }
 
-=cut
+  $self->{'miscSets'} ||= [];
 
-sub get_set_codes {
-  my $self = shift;
-  $self->{'sets'} ||= {};
-  return keys %{$self->{'sets'}};
+  push( @{$self->{'miscSets'}}, $miscSet );
 }
 
 
+
 =head2 get_set
 
-  Arg [1]    : string $code
+  Arg [1]    : optional string $code
                The code of the set to retrieve
   Example    : $set = $misc_feature->get_set($code);
   Description: Retrieves a set that this feature is associated with via its
@@ -200,16 +198,38 @@ sub get_set_codes {
 =cut
 
 sub get_set {
+  throw( "Use get_MiscSets()" );
+}
+
+
+=head2 get_all_MiscSets
+
+  Arg [1]    : optional string $code
+               The code of the set to retrieve
+  Example    : $set = $misc_feature->get_all_MiscSets($code);
+  Description: Retrieves a set that this feature is associated with via its
+               code. Can return empty lists. Usually returns about one elements lists.
+  Returntype : listref of Bio::EnsEMBL::MiscSet
+  Exceptions : throw if the code arg is not provided
+  Caller     : general
+
+=cut
+
+
+sub get_all_MiscSets {
   my $self = shift;
   my $code = shift;
 
-  throw('Code arg is required.') if (!$code);
-
-  return $self->{'sets'}->{$code};
+  $self->{'miscSets'} ||= [];
+  if( defined $code ) {
+    my @results = grep { uc($_->code())eq uc( $code ) } @{$self->{'miscSets'}};
+    return \@results;
+  } else {
+    return $self->{'miscSets'};
+  }
 }
 
 
-
 =head2 get_attribute
 
   Arg [1]    : string $type
@@ -223,20 +243,39 @@ sub get_set {
 =cut
 
 sub get_attribute {
+  throw( "Use get_Attributes now" );
+}
+
+
+=head2 get_all_Attributes
+
+  Arg [1]    : optional string $code
+               The code of the Attribute objects to retrieve
+  Example    : @attributes = $misc_feature->get_all_Attributes('name');
+  Description: Retrieves a list of Attribute objects for given code or all
+               of the associated Attributes.
+  Returntype : listref of Bio::EnsEMBL::Attribute
+  Exceptions : 
+  Caller     : general
+
+=cut
+
+sub get_all_Attributes {
   my $self = shift;
-  my $type = shift;
+  my $code = shift;
 
-  throw('Type arg is required.') if(!$type);
+  my @results;
+  my $result;
 
-  if(exists $self->{'attributes'}->{$type}) {
-    return wantarray() ? @{$self->{'attributes'}->{$type}} : $self->{'attributes'}->{$type}->[0];
+  if( defined $code ) {
+    @results = grep { uc( $_->code() ) eq uc( $code )} @{$self->{'attributes'}};
+    return \@results;
+  } else {
+    return $self->{'attributes'};
   }
-
-  return ();
 }
 
 
-
 =head2 display_id
 
   Arg [1]    : none
@@ -252,9 +291,13 @@ sub get_attribute {
 
 sub display_id {
   my $self = shift;
-  my ($attrib) = $self->get_attribute('name');
-  ($attrib) =  $self->get_attribute('synonym') if(!$attrib);
-  return $attrib || '';
+  my ($attrib) = @{$self->get_all_Attributes('name')};
+  ($attrib) =  @{$self->get_all_Attributes('synonym')} if(!$attrib);
+  if( defined $attrib ) {
+    return $attrib->value();
+  } else {
+    return '';
+  }
 }
 
 
diff --git a/modules/Bio/EnsEMBL/Slice.pm b/modules/Bio/EnsEMBL/Slice.pm
index 1286e5c6bf..d975be2edb 100644
--- a/modules/Bio/EnsEMBL/Slice.pm
+++ b/modules/Bio/EnsEMBL/Slice.pm
@@ -795,59 +795,44 @@ sub get_seq_region_id {
 }
 
 
-=head2 get_all_attribute_types
-
-  Arg [1]    : none
-  Example    : my @attribute_types = $slice->get_attribute_types();
-  Description: Gets a list of the types of attributes associated with the
-               seq_region this slice is on.
-  Returntype : list of strings
-  Exceptions : warning if slice does not have attached adaptor
-  Caller     : general
-
-=cut
-
-sub get_attribute_types {
-  my $self = shift;
-
-  if(!$self->adaptor()) {
-    warning('Cannot get attributes without an adaptor.');
-    return ();
-  }
-  my %attrib_hash = %{$self->adaptor->get_seq_region_attribs($self)};
-  return keys %attrib_hash;
-}
 
+=head2 get_all_Attributes
 
-=head2 get_attribute
-
-  Arg [1]    : string $attrib_type
+  Arg [1]    : optional string $attrib_code
                The code of the attribute type to retrieve values for.
-  Example    : ($htg_phase) = $slice->get_attribute('htg_phase');
-               @synonyms    = $slice->get_attribute('synonyms');
-  Description: Gets a list of values for a given attribute of this
-               slice''s seq_region.
-  Returntype : list of strings
-  Exceptions : warning is slice does not have attached adaptor
-               throw if argument is not provided
+  Example    : ($htg_phase) = @{$slice->get_all_Attributes('htg_phase')};
+               @slice_attributes    = @{$slice->get_all_Attributes()};
+  Description: Gets a list of Attributes of this slice''s seq_region.
+               Optionally just get Attrubutes for given code.
+  Returntype : listref Bio::EnsEMBL::Attribute
+  Exceptions : warning if slice does not have attached adaptor
   Caller     : general
 
 =cut
 
-sub get_attribute {
+sub get_all_Attributes {
   my $self = shift;
   my $attrib_code = shift;
 
-  throw('Attrib code argument is required.') if(!$attrib_code);
+  my $result;
+  my @results; 
 
   if(!$self->adaptor()) {
     warning('Cannot get attributes without an adaptor.');
-    return ();
+    return [];
   }
 
-  my %attribs = %{$self->adaptor->get_seq_region_attribs($self)};
+  my $attribute_adaptor = $self->db->get_AttributeAdaptor();
 
-  return @{$attribs{uc($attrib_code)} || []};
+  if( defined $attrib_code ) {
+    @results = grep { uc($_->code()) eq uc($attrib_code) }  
+      @{$attribute_adaptor->fetch_all_by_Slice( $self )};
+    $result = \@results;
+  } else {
+    $result = $attribute_adaptor->fetch_all_by_Slice( $self );
+  }
+
+  return $result;
 }
 
 
diff --git a/modules/t/miscFeature.t b/modules/t/miscFeature.t
index 5d29c0a4d9..62d020a119 100644
--- a/modules/t/miscFeature.t
+++ b/modules/t/miscFeature.t
@@ -3,10 +3,11 @@ use strict;
 
 BEGIN { $| = 1;
 	use Test ;
-	plan tests => 11
+	plan tests => 8
 }
 
 use MultiTestDB;
+use Bio::EnsEMBL::Attribute;
 use Bio::EnsEMBL::MiscFeature;
 use Bio::EnsEMBL::MiscSet;
 use TestUtils qw(debug test_getter_setter);
@@ -39,43 +40,47 @@ my $ms2 = Bio::EnsEMBL::MiscSet->new(4, undef,
 
 
 
-$mf->add_set($ms1);
-$mf->add_set($ms2);
+$mf->add_MiscSet($ms1);
+$mf->add_MiscSet($ms2);
 
 
-ok($mf->get_set($ms1->code) == $ms1);
-ok($mf->get_set($ms2->code) == $ms2);
+my $ms3 = $mf->get_all_MiscSets($ms1->code)->[0];
+my $ms4 = $mf->get_all_MiscSets($ms2->code)->[0];
 
-my @codes = $mf->get_set_codes;
+ok( $ms3 == $ms1);
+ok( $ms4 == $ms2);
 
-ok(@codes == 2);
-
-@codes = grep {($_ eq $ms1->code()) || ($_ eq $ms2->code())} @codes;
-
-ok(@codes == 2);
 
 #
 # Test add_attribute, get_attribute_types, get_attribute
 #
 
-my $name1 = 'test name';
-my $name2 = 'AL4231124.1';
+my $name1 = Bio::EnsEMBL::Attribute->new
+  ( -CODE => 'name',
+    -VALUE => 'test name'
+  );
+
+my $name2 = Bio::EnsEMBL::Attribute->new
+  ( -CODE => 'name',
+    -VALUE => 'AL4231124.1'
+  );
 
-$mf->add_attribute('name',  $name1);
 
-ok($mf->display_id eq $name1);
+$mf->add_Attribute( $name1 );
 
-$mf->add_attribute('name',  $name2);
-$mf->add_attribute('version', '4');
+ok($mf->display_id eq "test name");
 
-my @types = $mf->get_attribute_types();
-ok(@types == 2);
+$mf->add_Attribute( $name2 );
 
-@types = grep {$_ eq 'name' || $_ eq 'version'} @types;
+my $vers1 = Bio::EnsEMBL::Attribute->new
+  ( -CODE => 'version',
+    -VALUE => 4
+  );
 
-ok(@types == 2);
+$mf->add_Attribute( $vers1 );
 
-my @attribs = $mf->get_attribute('name');
+
+my @attribs = @{$mf->get_all_Attributes('name')};
 
 ok(@attribs == 2);
 
@@ -83,5 +88,8 @@ ok(@attribs == 2);
 
 ok(@attribs == 2);
 
-@attribs = $mf->get_attribute('version');
-ok(@attribs == 1 && $attribs[0] eq '4');
+@attribs = @{$mf->get_all_Attributes('version')};
+ok(@attribs == 1 && $attribs[0]->value() eq '4');
+
+@attribs = @{$mf->get_all_Attributes()};
+ok( @attribs == 3 );
diff --git a/modules/t/miscFeatureAdaptor.t b/modules/t/miscFeatureAdaptor.t
index 9868abaa0a..ec7a9f1ece 100644
--- a/modules/t/miscFeatureAdaptor.t
+++ b/modules/t/miscFeatureAdaptor.t
@@ -87,14 +87,12 @@ sub print_features {
   my $features = shift;
   foreach my $f (@$features) {
     if(defined($f)) {
-      my @attrib_types = $f->get_attribute_types;
-      my @attrib_vals  = map {"$_ => " . join(',',$f->get_attribute($_))} @attrib_types;
+      my @attribs = @{$f->get_all_Attributes() };
+      my @sets = @{$f->get_all_MiscSets()};
 
-      my @set_codes = $f->get_set_codes;
-      my @set_names = map {"$_ => " . $f->get_set($_)->name()} @set_codes;
-
-      my $attrib_string = join(":", @attrib_vals);
-      my $set_string = join(":", @set_names);
+      
+      my $attrib_string = join(":", map { $_->code()." => ".$_->value() } @attribs );;
+      my $set_string = join(":", map { $_->code() } @sets );
 
       my $seqname = $f->slice->seq_region_name();
       debug($seqname . ' ' . $f->start().'-'.$f->end().'('.$f->strand().
-- 
GitLab