[Bioperl-l] Feature/Annotation rollback finished

[Hilmar Lapp]

On Aug 28, 2007, at 5:05 PM, Chris Fields wrote:

> I'm now wrapping up the Feature/Annotation rollback.  I will probably
> start merging back to the main branch in the next day or two., as
> soon as interested parties (*cough*devs*cough*) look over the last
> batch of changes.
>
> http://www.bioperl.org/wiki/Feature_Annotation_rollback#Fourth_Round
>
> [...]
> It is also possible there are still some instances where overloading
> is expected lurking about in the ~1000 or so modules, so I'll leave
> the exceptions I added to all Bio::AnnotationI

Keep in mind that code such as

	if ($ann) { ... }

is mostly not b/c someone wanted to use overloading, but rather  
someone was lazy and really meant to say

	if (defined($ann)) { ... }

In the absence of eq overloading, these will behave identically. So  
if you leave the exceptions in it is sort-of policing lazy  
programmers, which I guess is fine in principle, but is guaranteed to  
trip up a lot of script code. I'd take it out if you're reasonably  
sure that at least within BioPerl itself those lazy programming  
incidents are removed.

> [...]
> The key change in this last round is the addition of several class
> *dbxref* methods to Bio::Ontology::Term and
> Bio::Annotation::OntologyTerm, all of which are capable of working
> with either DBLink instances or simple scalars.

I don't think you need the code here to deal with both scalars and  
objects. It is fine I think to define the new methods from the outset  
to consistently accept and return DBLink objects, and period.

The backwards compatibility logic should rather be in the *_dblink*()  
methods; i.e., instead of simple aliases they should have the code to  
map to and from the new API. That way, once the deprecation cycle  
ends, they can be removed, and with them all the legacy code that now  
is no longer needed, whereas if you have that in the new methods, it  
keeps bothering the maintainers.

You also mention a add_dbxref_context() on the wiki page - I'm not  
sure why that would be needed given that you build in the -context  
option to add_dbxref() from the outset. But maybe I've glossed over  
some detail.

Once this is merged back to the main trunk, I guess we need to give  
Bio::SeqFeature::TypedSeqFeatureI a thorough look and make sure it  
makes real sense.

Thanks Chris for this effort, this clears a monumental roadblock.

	-hilmar
-- 
===========================================================
: Hilmar Lapp  -:-  Durham, NC  -:-  hlapp at gmx dot net :
===========================================================