[Bioperl-l] Compliance of Bio::Seq add_SeqFeature() method

[Fields, Christopher J]

Beyond the grammatical incorrectness of passing multiple features to a method named add_SeqFeature(), I'm fairly neutral on it, actually, as long as the behavior is (1) consistent and (2) documented (seems the interface itself would need some clarification with that in mind).

chris

On Mar 6, 2012, at 10:51 PM, Hilmar Lapp wrote:

> Accepting an array is not in violation so long as the method also accepts a single feature (i.e., a single object ref), and so long as passing an array isn't going to choke if it is followed by 'EXPAND'. I wouldn't deprecate this otherwise.
> 
> 	-hilmar
> 
> On Mar 7, 2012, at 12:00 AM, Florent Angly wrote:
> 
>> On 05/03/12 12:13, Fields, Christopher J wrote:
>>> Actually, I misread Florent's original post, I was thinking that FeatureHolderI was the outlier here, but it is Bio::Seq.  Yes, I think Bio::Seq is abusing the FeatureHolderI interface, it should just be for a single feature (it can safely ignore the 'EXPAND' option).  However, use of 'EXPAND' assumes the FeatureHolderI is also a Bio::RangeI (must have a start and end to expand), something that is not mentioned in the interface as a requirement and is not guaranteed, for instance Bio::Seq is not Bio::RangeI.
>> Ok, I have:
>> 1/ clarified in Bio::FeatureHolderI that there is no guarantee that 'EXPAND' will be honored
>> 2/ made Bio::Seq comply to Bio::FeatureHolderI by accepting the 'EXPAND' keyword (but do nothing about it)
>> 3/ deprecated the use of passing multiple features to add_SeqFeature() in Bio::Seq
>> 4/ updated documentation and code that relied on passing multiple features
>> 
>> That should take care of the issue at hand. See this commit: https://github.com/bioperl/bioperl-live/commit/a5bebe00c505fbf5279f5d717790ed36eefcc2b8
>> 
>> Note that there are still some modules (e.g. Bio::DB::SeqFeature::Store, Bio::DB::SeqFeature::NormalizedFeature, Bio::SeqFeature::Lite, Bio::DB::SeqFeature, Bio::Search::Tiling::MapTileUtils) that have an add_SeqFeature() method that accepts an array of features but they are not Bio::FeatureHolderI, so that's ok. Maybe they should be Bio::FeatureHolderI but that's another story.
>> 
>> However, I have found that Bio::SimpleAlign is a Bio::FeatureHolderI and is not compliant. I fixed it here: https://github.com/bioperl/bioperl-live/commit/29e0449d05f37c9c748aaaff8cfe596ca7c3d380
>> 
>> Florent
> 
> -- 
> ===========================================================
> : Hilmar Lapp -:- Durham, NC -:- hlapp at drycafe dot net :
> ===========================================================
> 
> 
> 
>