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

[Ben Bimber]

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





On Thu, Jun 10, 2010 at 9:58 AM, Chris Fields <cjfields at illinois.edu> wrote:
> On Jun 10, 2010, at 8:43 AM, Ben Bimber wrote:
>
>> hello,
>>
>> a little while ago i posted a bug about commandexts when you pass an
>> array ref.  when you pass an arrayref of files, instead of a string
>> with 1 file, you get an error saying 'cannot use string '......' as an
>> array ref while strict refs in place'.  i believe i understand the
>> bug, but am not entirely sure the best way to fix it.  here's the
>> original code from commandexts.pm, starting line 989:
>>
>>    my @files = @args{@specs};
>>    # expand arrayrefs
>>    my $l = $#files;
>>    for (0..$l) {
>>       if (ref($files[$_]) eq 'ARRAY') {
>>           splice(@files, $_, 1, @{$files[$_]});
>>           splice(@switches, $_, 1, ($switches[$_]) x @{$files[$_]});
>>       }
>>    }
>>
>> It throws the error on the second splice().  The reason is b/c
>> $files[$_] isnt an array ref anymore, b/c we just expanded it in the
>> line before.  Truthfully, I dont entirely understand why we're
>> expanding out @switches for each file.  Maybe i'm missing something
>> obvious, but why would we want these copied?
>
> Maybe @file and @switches are in-sync and thus need similar expansion?  Not sure, personally.
>
>> however, there's couple ways around it:
>>
>> 1. save the value of  @{$files[$_]} by adding: my @tmp =
>> @{$files[$_]};, then using @tmp on that line
>> 2. put the second splice() before the first
>> 3. if the idea is actually to expand @switches against every file,
>> then move this outside the for loop
>
> The first one seems safest.  The real question is, do any of these solutions work?  Do any of the modules requiring the command extensions in bioperl-run show problems, for instance?
>
>> I originally posted that this could be fixed by surrounding the
>> problem line with 'no strict'/ 'use strict'; however, that really only
>> worked b/c the command I was using did not actually have any switches.
>> I doubt it would work properly if your command had them.
>>
>> -Ben
>
> I think, if one of the proposed solutions works, pick which you think is the best and go with it.  Test it out, then we can commit this to github.
>
> BTW, sorry about the warnocking, a number of us (myself included) have been extremely busy as of late.
>
> chris
> _______________________________________________
> Bioperl-l mailing list
> Bioperl-l at lists.open-bio.org
> http://lists.open-bio.org/mailman/listinfo/bioperl-l
>