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

[Florent Angly]

I realize that Hilmar. However, to me,  it makes it clearer to add a 
single feature and it leaves room to implement a add_SeqFeatures() 
method in FeatureHolderI to add multiple features at once, based on the 
implementation-specific add_SeqFeature() method.

I also realize that deprecation should be used with caution. Feel free 
to revert and let add_SeqFeature() accept an array of features if you 
think that the downsides (deprecating arrays) outweights the advantages.

Florent


On 07/03/12 14:51, 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