Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've hodgepodged together an attempt to extract all phone numbers from all CSVs in a directory, regardless of where they are and what format they're in. I want all phone numbers to be printed to a compilation CSV with no repeat elements in the format 8885551234 with NO leading ones. E.g., 1 (510) 533-2134 would become 5105332134, as would 510-5332134 (and the latter would count as a dupe and fail to be written).

My challenges are that the numbers are in different formats, in different columns (and sometimes in several columns), and that the files in the directory are, in total, many gigabytes in size.

What I came up with, first, fails to weed out numbers that start with 0 or 1 (though it does seem to "skip" the 1 in 11-digit numbers); second, I need checked by a regexpert to see if it is even doing what I want (identifying and printing phone numbers); and third, is far too slow:

import csv
import re
import glob
import string

with open('phonelist.csv', 'wb') as out:
    seen = set()
    output = []
    out_writer = csv.writer(out)
    csv_files = glob.glob('*.csv')
    #csv_files2 = glob.glob('*.csv')
    for filename in csv_files:
        with open(filename, 'rbU') as ifile:
            read = csv.reader(ifile)
            for row in read:
                for column in row:
                    s1 = column.strip()
                    result = re.match(
                        r'.*(\+?[2-9]?[0-9]?[0-9]?-?\(?[0-9][0-9][0-9]\)? ?[0-9][0-9][0-9]-?[0-9][0-9][0-9][0-9]).*', s1) or re.match(
                        r'.*(\+?[2-9]?[0-9]?[0-9]?-?\(?[0-9][0-9][0-9]\)?-?[0-9][0-9][0-9]-?[0-9][0-9][0-9][0-9]).*', s1)
                    if result:
                        tempStr = result.group(1)
                        for ch in ['(', ')', '-', ' ']:
                            tempStr = tempStr.replace(ch, '')
                        if tempStr not in seen:
                            seen.add(tempStr)
                            output.append(tempStr)
    for val in output:
        out_writer.writerow([val])
share|improve this question
    
You say that you intend to strip out the country code, but I don't see that happening in the code. –  200_success Apr 15 at 5:49

2 Answers 2

up vote 4 down vote accepted

Use a compiled regular expression.

Since you are looking for the pattern anywhere within a string, use re.search() instead of re.match(). Actually, the behaviour is not exactly the same. .search() will return the leftmost match, whereas .match() using your original pattern will return the rightmost match, since the initial .* is greedy. The backtracking that is necessary to accomplish a rightmost match would be a source of inefficiency.

By using .search(), which performs a search not anchored to the start of the subject string, you can also eliminate the capturing parentheses and use .group(0) instead.

You can also shorten the regex by using \d (match any digit) and {n} (match the previous element n times). Since your two regular expressions differ by only one character, combine them, using a [ -] character class to cover both cases.

In the canonicalization step, you want to eliminate all non-digits, so just do that instead.

regex = re.compile(r'\+?[2-9]?\d?\d?-?\(?\d{3}\)?[ -]?\d{3}-?\d{4})')
…

match = regex.search(s1)
if match:
    canonical_phone = re.sub(r'\D', '', match.group(0))
    if canonical_phone not in seen:
        seen.add(canonical_phone)
        output.append(canonical_phone)
share|improve this answer
    
This takes about 20% the time of my original script! That's exciting. However, it includes numbers that start with 1, so that the two numbers mentioned above come out as 5105332134 and 15105332134, and therefore as dupes (I want the latter to output the same way as the former, and it looks from the compiled regex like it should only start with 2-9, right?) –  Xodarap777 Apr 15 at 6:50
    
As noted in my comment on the question, I don't see that you tried to strip out the country code anywhere in your original program. I just reproduced the bug, that's all. It should be trivial to fix, though, using a capturing pair of parentheses. –  200_success Apr 15 at 7:35

Simple comments here :

  • You don't need both seen and output: the set will be enough to store the unique numbers and print them later on.

  • You should keep your reading logic and your writing logic independent by performing the former and then the latter. That would make things clearer and remove an unneeded level of nesting.

  • Probably because I still have my morning eyes but I can't see the difference between the two regexps.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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