[Bioperl-l] commandexts and array refs bug - found problem, not sure of best fix

Chris Fields cjfields at illinois.edu
Thu Jun 10 17:04:08 UTC 2010


On Jun 10, 2010, at 11:46 AM, Ben Bimber wrote:

> i suspect this piece of code is seldom encountered.  if any existing
> bioperl wrapper actually tried to pass an arrayref to commandexts, it
> would hit this same error.  i only encountered this using samtools
> merge.
> 
> I looked a little more into exactly what @switch is doing.  it seems
> to allow you to supply switches specific to a given file.  the bowtie
> wrapper uses it with paired end data (the first fasta gets a -1 and
> and second gets a -2).  however, i dont see anywhere where this
> actually applies in a multi-file situation.  nonetheless, reversing
> the order of the two existing splice() (lines 991-992) I think will
> preserve whatever functionality was there and prevent the error:
> 
>    my @files = @args{@specs};
>    # expand arrayrefs
>    my $l = $#files;
>    for (0..$l) {
>       if (ref($files[$_]) eq 'ARRAY') {
>           splice(@switches, $_, 1, ($switches[$_]) x @{$files[$_]});
>           splice(@files, $_, 1, @{$files[$_]});
>       }
>    }
> 
> As I write this email and look at the code above a little more, I
> think that the current approach used to expand these arrayrefs will
> also break down in other situations, including when the arrayref is
> not in the last element of the @files array or when 2 arrayrefs are
> passed.  However, it seems that this is barely used and it really
> depends how much time/thought you want to put into it.
> 
> -Ben

It's worth documenting this concern in the code as a comment, particularly if the situation does pop up at some point.

Do you want to send a patch in?  Or just want us to make the switch in the code directly?

chris






More information about the Bioperl-l mailing list