[Bioperl-l] Bio::SeqFeature::Annotated API change

[Chris Fields]

On Oct 11, 2007, at 5:40 AM, Sendu Bala wrote:

> Following Chris's changes to SF::Annotated et al., lots of existing  
> user
> code breaks. Also, at least some Bioperl code breaks, notably
> Bio::DB::SeqFeature::Store, which in mysql mode calls
> _get_location_and_bin() which calls $feature->seq_id which ends up
> storing something like 'Bio::Annotation::SimpleValue=HASH 
> (0x1f435d0)' in
> the database, instead of an actual sequence id (which completely  
> breaks
> searching by seq_id).
>
> I propose its API be changed to be more consistent with
> Bio::SeqFeatureI, eg. instead of:
>
> seq_id()
>   Usage   : $obj->seq_id($newval)
>   Function: holds a string corresponding to the unique
>             seq_id of the sequence underlying the feature
>             (e.g. database accession or primary key).
>   Returns : a Bio::Annotation::SimpleValue object representing the
>             seq_id.
>   Args    : on set, some string or a Bio::Annotation::SimpleValue  
> object.
>
> we have:
>
> seq_id()
>   Usage   : $obj->seq_id($newval)
>   Function: holds a string corresponding to the unique
>             seq_id of the sequence underlying the feature
>             (e.g. database accession or primary key).
>   Returns : string representing the seq_id.
>   Args    : on set, some string or a Bio::Annotation::SimpleValue  
> object.
>
> This would apply to seq_id(), name(), type(), source(), phase() and
> frame(). Internally the implementation could store the string value  
> in a
> SimpleValue object.

Agreed.  It would be easy to change over but we need to also make  
sure FeatureIO fixes are in place.  In reality all FeatureIO methods  
should be changed over to recognize any SeqFeatureI or (if we retain  
it) the stricter TypedSeqFeatureI.  Using only Bio::SF::Annotated  
limits other more lightweight implementations.  I simply haven't had  
time to work on it yet due to $job; if you want to make the necessary  
changes you are more than welcome; the few tests I found I moved into  
SeqFeatAnnotated.t, which likely expects the wrong behavior.

> However, I'm obviously missing something, because I have no idea what
> the justification for returning SimpleValue objects was in the first
> place (what other module needs them?), nor even what the point of
> SimpleValue objects is in the first place.

I believe it was to ensure any data stored or retrieved was strongly  
typed (i.e. scalars in SimpleValue, dbxrefs in DBLink, comments in  
Comment, etc).  Since B::SF::Generic is also AnnotatableI, it can  
store a mix of scalars in methods as well as Bio::Annotation data;  
this class attempts to lump them all together as Bio::Annotation in a  
Collection in a strongly typed, uniform way.  Hilmar's  
Bio::SF::AnnotationAdaptor frankly does a better job of describing  
the reasoning behind this and is more flexible; I use that now for  
typing via feature(), though I just realized it should be changed to  
be a singleton instance per class (oops!).

B::SF::A violated the SeqFeatureI interface from the get-go by  
returning objects.  To trick it's way around the issue it used  
overloading so that calling it in some contexts (print, comparison)  
returned a string or value; removing the overloads unmasked that  
behavior.  To me an object returned (regardless of overloading) is  
still an object and not a scalar, and still violates the interface  
methods where scalars are expected.  I can't fault the authors  
involved completely since the idea was to radically change the way  
SFs/Annotation worked together, but the implementation was never  
completed so I rolled it back and limited typing to B::SF::A until  
something else can be worked out.

Personally I think it's too 'heavy' and other options should be  
explored, such as abstracting out the type checking into a separate  
utility class which FeatureIO can use on any SeqFeatureI (TypeMapper  
does something like this for the primary_tag()).

chris