Removing diacritics from characters alters the data you're parsing. Are you supposed to do that? Probably not. You certainly shouldn't have to modify the data for any reason.

What character encoding is the text in? ISO 8859-1 (Latin 1)? Windows-1252? UTF-8? If you don't know, I encourage you to find out. You really ought to know.

I suspect your few problems — malformed CSV records and text that isn't in the ASCII character encoding — can be solved by using Text::CSV::Encoded and also enabling allow_loose_quotes as others have recommended.

UPDATE: Using Text::CSV::Encoded may be overkill. Jenda's recommendation to set the binary attribute to true (1) may be all you really need. But I nonetheless still believe you ought to know what character encoding the text is in.

Good point. It would be better to avoid removing the diacritics if possible. I did find that setting the binary attribute to true for Text::CSV did prevent the script from choking on the diacritics.

I do not know the character encoding and don't know how to identify it, so I asked Mr. Google (Mr. Google knows all!) and am looking through what he dredged up. I looked at the documentation in CPAN for Text::CSV::Encoded. It appears that, in order to use this, you need to know the enocoding for the input and what you want to use for the output.

I seem to have it working fairly well except for some of the cases where the weight is "" or 0.0. I haven't yet figured out why it works correctly for some records and not for others. I will look at the records where it fails and see if I can identify that.

Thanks very much.

I refactored your script. Most of the changes I made are ones recommended in Perl Best Practices by Damian Conway. The revised script is untested and intended merely to suggest coding improvements.

#!/usr/bin/perl

use strict;
use warnings;

use English qw( -no_match_vars );
use Text::CSV;

my $release_file = '../ecodata/releases.txt';

open my $release_fh, '<', $release_file
    or die "Can't open release file $release_file: $OS_ERROR\n";

my $csv = Text::CSV->new({
    allow_loose_quotes => 1,
    binary             => 1,
});

CSV_RECORD:
while (my $csv_record = <$release_fh>) {
    # Skip header...
    next CSV_RECORD if $INPUT_LINE_NUMBER == 1;

    # SPECIAL CASE: Remove unwanted text...
    $csv_record =~ s/ \(\d{4} and after "acid aerosols" only\)//g;

    if (not $csv->parse($csv_record)) {
        my $csv_error_input = $csv->error_input();

        warn "Can't parse CSV record at line $INPUT_LINE_NUMBER"
           . " of release file $release_file:\n$csv_error_input\n";

        next CSV_RECORD;
    }

    # TRI, Release#, ChemName, RegNum, Year, Pounds, Grams
    my ($tri, undef, $chem_name, undef, $year, $pounds, $grams)
        = $csv->fields();

    # [Do what you need to do with the data here]
}

close $release_fh;

exit 0;
[download]

I moved the Text::CSV->new() constructor outside the while loop. I moved the lexical variables inside the while loop. I added a space to the beginning of the regular expression pattern matching the string being globally removed from the CSV records.

I won't inflict the entire script on you, just the pertinent code snippet. The snippet I posted before now looks like this, with the old code commented out:

while  <RLS> )
{
#        $_ = undiacritic($_);
#        s/\(\d{4} and after \"acid aerosols\" only\)//g;
#        $csv = Text::CSV->new();
         $csv = Text::CSV->new (
            { allow_loose_quotes     => 1 ,
              escape_char            => "\\",
              binary                 => 1, 
            }
         ) or die "" . Text::CSV->error_diag ();
        next if ($. == 1);
        if ($csv->parse($_)) {
                @cols = $csv->fields();
                $tri    = $cols[0];
                $chem   = $cols[2];
                $year   = $cols[4];
                $lbs    = $cols[5];
                $gms    = $cols[6];
        }
        else {
                $err = $csv->error_input;
                print "Failed to parse line: $err";
        }
}
close(RLS);
[download]

allow_loose_quotes did the trick on the embedded quotes - I found something that said to change the escape_char so it's not the same as the quote_char, so I did that as well. binary=>1 eliminated the need for undiacritic().

Now, instead of processing each record three times, I'm processing it once. With 1.7 million records, that is very nice.

Well, I thought I was done. It turns out that some of the fields (for lbs and gms) are "", some are 0.0, and some are something like 123.4 in the input file. I changed the assignment to this:

if (!$cols[5]) { $lbs = 0 } elsif ($cols[5] eq "0.0") { $lbs = 0 } els
+e { $lbs = $cols[5] }
if (!$cols[6]) {$gms = 0} elsif ($cols[5] eq "0.0") { $lbs = 0 } else 
+{ $gms = $cols[6] }
[download]

and then this test

gives valid results. Is there a better way to do this? It seems rather clunky. I'd have thought that 0.0 would be interpreted the same as 0, but apparently not.

Thanks so much for your help.

Code like

while (<RLS>)
{
    $csv = Text::CSV->new (
            { allow_loose_quotes     => 1 ,
              escape_char            => "\\",
              binary                 => 1, 
            }
         ) or die "" . Text::CSV->error_diag ();
        next if ($. == 1);
        if ($csv->parse ($_)) {
[download]

will very soon start failing when the fields have embedded newlines, as <RLS> doesn't see the quotation and escapes. When parsing CSV with Text::CSV or Text::CSV_XS, please use getline (). Also note that in your example you create a new csv parsing object for every line read. You should bring the object outside the loop. If possible, choose encoding on the lowest possible level: at open () time.

my $csv = Text::CSV_XS->new ({
    binary             => 1,
    escape_char        => "\\",
    allow_loose_quotes => 1,
    auto_diag          => 1,
    });
open my $fh, "<:encoding(utf-8)", "text.csv" or die "text.csv: $!";
$csv->getline ($fh); # skip first line
while (my $row = $csv->getline ($fh)) {
    my ($tri, $chem, $year, $lbs, $gms) = 
        map { undiacritic ($_) } @{$row}[0,2,4,5,6];
    :
    :
    }
close $fh;
[download]


Enjoy, Have FUN! H.Merijn

Once again, I strongly recommend you move the object constructor outside the while loop. Tux, a.k.a. H. Merijn Brand, the author of Text::CSV_XS, which is the base module you're indirectly using, amplified this suggestion in his reply to your post. He explained that "you create a new csv parsing object for every line read." You really don't want to do this needlessly for 1.7 million records. And Tux's advice to use getline instead of <>/parse/fields is a wise one.

I think you're trying too hard to handle the floating-point weight values. Just be explicit with your tests:

if ($pounds == 0.0 and $grams == 0.0) {
    ...
}
[download]

The right coercions happen automagically. Here's the proof:

D:\>perl -E "say '' == 0.0 ? 'Equals' : 'Does not equal'"
Equals

D:\>perl -E "say '0' == 0.0 ? 'Equals' : 'Does not equal'"
Equals

D:\>perl -E "say '0.0' == 0.0 ? 'Equals' : 'Does not equal'"
Equals

D:\>perl -E "say '123' == 0.0 ? 'Equals' : 'Does not equal'"
Does not equal

D:\>perl -E "say '123.4' == 0.0 ? 'Equals' : 'Does not equal'"
Does not equal

D:\>
[download]

So you didn't like any of the other improvements I suggested in my refactored version of your script?

Jim

I've incorporated Tux's suggestion to use getline (getline_hr, actually) instead of <>/parse/fields. It really tightens up the whole script.

#!/usr/bin/perl

use strict;
use warnings;

use English qw( -no_match_vars );
use Text::CSV;

$OUTPUT_FIELD_SEPARATOR  = "\n";
$OUTPUT_RECORD_SEPARATOR = "\n";

my $release_file = '../ecodata/releases.txt';

# Text is in the ISO 8859-1 (Latin 1) encoding
open my $release_fh, '<:encoding(iso-8859-1)', $release_file
    or die "Can't open release file $release_file: $OS_ERROR\n";

my $csv = Text::CSV->new({
    auto_diag          => 1,
    binary             => 1,
    allow_loose_quotes => 1,
    escape_char        => '\\',
});

# Header is 'TRI,Release#,ChemName,RegNum,Year,Pounds,Grams'
$csv->column_names($csv->getline($release_fh));

while (my $value = $csv->getline_hr($release_fh)) {
    {
        no warnings qw( numeric );

        if ($value->{'Pounds'} == 0.0 and $value->{'Grams'} == 0.0) {
            warn "Release $value->{'Release#'} is weightless\n";
        }
    }

    print $value->{'TRI'},
          $value->{'Release#'},
          $value->{'ChemName'},
          $value->{'RegNum'},
          $value->{'Year'},
          $value->{'Pounds'},
          $value->{'Grams'};
}

close $release_fh;

exit 0;
[download]

The bind_columns () method is actually faster. It matters when your streams are big

my $csv = Text::CSV->new ({
    auto_diag          => 1,
    binary             => 1,
    allow_loose_quotes => 1,
    escape_char        => "\\",
    });

# Header is 'TRI,Release#,ChemName,RegNum,Year,Pounds,Grams'
my %value;
$csv->bind_columns (\@value{@{$csv->getline ($release_fh)}});
while ($csv->getline_hr ($release_fh)) {
    {   no warnings "numeric";
        $value{Pounds} == 0.0 && $value->{Grams} == 0.0 and
            warn "Release $value->{'Release#'} is weightless\n";
        }

    print $value{"TRI"},
          $value{"Release#"},
          $value{"ChemName"},
          $value{"RegNum"},
          $value{"Year"},
          $value{"Pounds"},
          $value{"Grams"};
    }
[download]

YMMV, bench to check if it also validates for your set of data. My speed comparison looks like this. In that image, the lower the line, the faster, so Text::CSV_XS with bindcolumns () (labeled "xs bndc") is the fastest on all sizes and the pure perl Text::CSV_PP counterpart with bindcolumns () (labeled "pp bndc") is the slowest, as it has the most overhead in pure perl. If you only look at the differences in the XS implementation, look at this graph.

Update 1: removed the erroneous call to column_names () as spotted by jim.

Update 2: New graphs: XS + PP and XS only


Enjoy, Have FUN! H.Merijn