4

I am a brand new Perl novice, looking for help with my first ever Perl script

I have some huge files 30-50GB files and they are constructed like this - millions of columns and thousands of rows:

A B C D E 1 2 3 4 5 6 7 8 9 10 
A B C D E 1 2 3 4 5 6 7 8 9 10 
A B C D E 1 2 3 4 5 6 7 8 9 10 
A B C D E 1 2 3 4 5 6 7 8 9 10 
A B C D E 1 2 3 4 5 6 7 8 9 10 
A B C D E 1 2 3 4 5 6 7 8 9 10 
A B C D E 1 2 3 4 5 6 7 8 9 10

I would like to delete column "A", and column "C", then ever third of the number columns, so the "3" column and the "6" column, then "9" column until the end of the file. Space delimited.

My attempt is like this:

#!/usr/local/bin/perl

use strict;
use warnings;

    my @dataColumns;
    my $dataColumnCount;
    if(scalar(@ARGV) != 2){
        print "\nNo files supplied, please supply file name\n";
        exit;
    }

    my $Infile = $ARGV[0];
    my $Outfile = $ARGV[1];

    open(INFO,$Infile) || die "Could not open $Infile for reading";
    open(OUT,">$Outfile") || die "Could not open $Outfile for writing";

    while (<INFO>) {
        chop;
        @dataColumns = split(" ");
        $dataColumnCount = @dataColumns + 1;
#Now remove the first element of the list
        shift(@dataColumns);

#Now remove the third element (Note that it is now the second - after removal of the first)
        splice(@dataColumns,1,1); # remove the third element (now the second)

#Now remove the 6th (originally the 8th) and every third one thereafter
#NB There are now $dataColumnCount-1 columns

        for (my $i = 5; $i < $dataColumnCount-1; $i = $i + 3 ) {
            splice($dataColumns; $i; 1);
        }

#Now join the remaining elements of the list back into a single string
        my $AmendedLine = join(" ",@dataColumns);

#Finally print out the line into your new file
        print OUT "$AmendedLine/n";
}

But I am getting a few weird errors:

  1. It is saying it doesn't like my $1 in the for loop, I have added a 'my' which seems to make the error go away but nobody else's for code seems to contain a 'my' here so I am not sure what is going on.

Global symbol "$i" requires explicit package name at Convertversion2.pl line 36. Global symbol "$i" requires explicit package name at Convertversion2.pl line 36. Global symbol "$i" requires explicit package name at Convertversion2.pl line 36. Global symbol "$i" requires explicit package name at Convertversion2.pl line 36.

  1. The other error is this: syntax error at Convertversion2.pl line 37, near "@dataColumns;" syntax error at Convertversion2.pl line 37, near "1)"

I am not sure how to correct this error, I think I am almost there, but not sure what exactly what the syntax error is, is am unsure how to fix it.

Thank you in advance.

7
  • You had a little brainfart when you wrote the splice line. First off, $dataColumns should be an array @dataColumns. Second of all, you should use commas for lists, not semi-colons. Instead of using splice, you can just use an array slice, e.g. print OUT join(" ", @dataColumns[1,3,4,5,6,8,9])
    – TLP
    Aug 23, 2013 at 0:10
  • @TLP: A slice isn't exactly practical for "millions of columns"!
    – Borodin
    Aug 23, 2013 at 0:15
  • $dataColumnCount becomes increasingly inaccurate as you splice. Do you know what @#dataColumns represents? How does the result of the splice affect the validity of your for loop's increment clause?
    – tjd
    Aug 23, 2013 at 0:18
  • @Borodin It is as easy to make a list of indexes to keep as it is to make a list of indexes to delete. And it is certainly preferable to juggling splice around with loops and whatnot.
    – TLP
    Aug 23, 2013 at 0:19
  • @TLP: Ah so you were thinking more of print join(' ', @dataColumns[@keep]), "\n". Maybe. I would be interested to know whether that is at all efficient for large @keep.
    – Borodin
    Aug 23, 2013 at 0:26

3 Answers 3

3

After I blogged about this question, a commenter pointed out that it is possible to reduce run time by 45% for my test case. I paraphrased his code a little bit:

my @keep;
while (<>) {
    my @data = split;

    unless (@keep) {
        @keep = (0, 1, 0, 1, 1);
        for (my $i = 5; $i < @data; $i += 3) {
            push @keep, 1, 1, 0;
        }
    }

    my $i = 0;
    print join(' ', grep $keep[$i++], @data), "\n";
}

This runs in almost half the time my original solution took:

$ time ./zz.pl input.data > /dev/null 
real 0m21.861s
user 0m21.310s
sys 0m0.280s

Now, it is possible to gain another 45% performance by using Inline::C in a rather dirty way:

#!/usr/bin/env perl

use strict;
use warnings;

use Inline C => <<'END_C'

/*
  This code 'works' only in a limited set of circumstances!
  Don't expect anything good if you feed it anything other
  than plain ASCII 
*/

#include <ctype.h>

SV *
extract_fields(char *line, AV *wanted_fields)
{
    int ch;
    IV current_field = 0;
    IV wanted_field = -1;

    unsigned char *cursor = line;
    unsigned char *field_begin = line;
    unsigned char *save_field_begin;

    STRLEN field_len = 0;
    IV i_wanted = 0;
    IV n_wanted = av_len(wanted_fields);

    AV *ret = newAV();
    while (i_wanted <= n_wanted) {
        SV **p_wanted = av_fetch(wanted_fields, i_wanted, 0);
        if (!(*p_wanted)) {
            croak("av_fetch returned NULL pointer");
        }
        wanted_field = SvIV(*p_wanted);

        while ((ch = *(cursor++))) {

            if (!isspace(ch)) {
                continue;
            }

            field_len = cursor - field_begin - 1;
            save_field_begin = field_begin;
            field_begin = cursor;

            current_field += 1;
            if (current_field != wanted_field) {
                continue;
            }

            av_push(ret, newSVpvn(save_field_begin, field_len));
            break;
        }
        i_wanted += 1;
    }
    return newRV_noinc((SV *) ret);
}

END_C
;

And, here is the Perl part. Note that we split only once to figure out the indices of fields to keep. Once we know those, we pass the line and the (1-based) indices to the C routine to slice and dice.

my @keep;
while (my $line = <>) {
    unless (@keep) {
        @keep = (2, 4, 5);
        my @data = split ' ', $line;
        push @keep, grep +(($_ - 5) % 3), 6 .. scalar(@data);
    }
    my $fields = extract_fields($line, \@keep);
    print join(' ', @$fields), "\n";
}
$ time ./ww.pl input.data > /dev/null 
real 0m11.539s
user 0m11.083s
sys 0m0.300s

input.data was generated using:

$ perl -E 'say join(" ", "A" .. "ZZZZ") for 1 .. 100' > input.data

and it is about 225MB in size.

2
  • This works and is substantially faster (the first code you give, at least). Argggggg.... I can only vote on one answer, I don't want to not be appreciative
    – Rudolph
    Aug 23, 2013 at 1:02
  • Well, I thought my answer would be substantially faster than @Borodin's given that his does many, many splices per line. For a line with, say, 3 million columns, we are talking about one million splices and splice is not known to be the fastest thing on earth in the first place. I already blogged about this. Thanks for changing your vote. Aug 23, 2013 at 1:16
2

The code you show doesn't produce those errors. You have no $1 in there at all, and if you meant $i then your use of that variable is fine. The only syntax error is in the line splice($dataColumns; $i; 1) which has semicolons instead of commas, and uses $dataColumns instead of @dataColumns.

Apart from that

  • It is good practice to declare variables as close as possible to their point of use, not at the top of the program.

  • Capital letters are generally used for constants like package names. You should use lower case, digits and underscore for variables.

  • Are you aware you are setting $dataColumnCount to one more than the number of elements in @dataColumns?

  • It is frowned on more recently to use global file handles - you should use lexical variables instead.

I suggest this refactoring of your program. It uses autodie to avoid having to check the success of the open calls. It builds a list of array indices that need deleting as soon as it can: once the number of fields in each line is known after the first record is read. Then it deletes them from the end backwards to avoid having to do arithmetic on the indices as preceding elements are removed.

#!/usr/local/bin/perl

use strict;
use warnings;
use autodie;

if (@ARGV != 2) {
  die "\nNo files supplied, please supply file names\n";
}

my ($infile, $outfile) = @ARGV;
open my $info, '<', $infile;
open my $out,  '>', $outfile;

my @remove;

while (<$info>) {

  my @data = split;

  unless (@remove) {
    @remove = (0, 2);
    for (my $i = 7; $i < @data; $i += 3) {
      push @remove, $i;
    }
  }

  splice @data, $_, 1 for reverse @remove;

  print $out join(' ', @data), "\n";
}
2
  • This certainly works very well, thank you, I will study this alternate method, and try to understand it, so I can replicate it. Appreciate your help very much.
    – Rudolph
    Aug 23, 2013 at 0:37
  • Just FYI: For a line with millions of columns, your code will involve a lot of splices. Aug 23, 2013 at 1:42
0

While the other answers above work perfectly, and mine probably doesn't present any advantage, this is a different way of achieving the same while avoiding split:

#!/usr/local/bin/perl
use strict;
use warnings;
use feature 'say';

my $dir='D:\\';
open my $fh,"<", "$dir\\test.txt" or die;

while (<$fh>) {
    chomp;
    my @fields = split ' ';
    print "$fields[0] $fields[2] ";
    for (my $i=7; $i <= $#fields; $i += 3){
        print "$fields[$i] ";
    }
    print "\n";
}
close $fh;

Please let me know if this is useless.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.