Really, what I would like to know is: "What does this script tell you about how I need to improve as a programmer?" I'm somewhat new to both Python and programming, so feel free to minimize your assumptions about my knowledge.
The purpose of this script is to read a csv of names and emails addresses that are improperly organized -- sometimes there is only one name, sometimes three names (first, middle, last) are in the same cell, and sometimes the person has a title (Mr., Mrs.) with their name.
Unfortunately, I can't post pictures yet, so here's a link to an example of what a bad row might look like, and how it should look.
Right now, the code does "work". It will execute, but there's a few small issues with some of these functions -- for example, the names aren't properly distributed to columns before the file is written. I'm less interested in these smaller things right now, and would rather have a bigger picture review.
I'll happily accept whatever advice you'll offer, but I'd most appreciate feedback on how I've accepted command line arguments, and the layout of the "engine" that controls the flow of operation and calls functions, and specific changes to how I've written the internals of each function -- for example, how could they be faster?
I've also posted general questions I've had along the way. If you're interested, please weigh in on those (see below code). I'm also less interested in unanticipated cases, but welcome them if you're inspired.
There's also specific in-line questions at the bottom that are referenced with [qx].
import csv import sys # This script will expext three arguments: # 1. File with data to be scrubbed (CleanCsv.in_file) # 2. File name to output clean data (CleanCsv.clean_out_file) # 3. File name to output data that's needs review (CleanCsv.dirty_out_file) class CleanCsv(object): def __init__(self, args): self.in_file = args # Storing original (necc?) self.clean_out_file = args self.dirty_out_file = args self.flags = [x for x in args[4:] if sys.argv != ""] # Unnecessary? sys.argv will never be blank---> ^^^ self.manual_repair =  self.rows =  self.functions = [ 'strip_blank_fields', 'strip_whitespace', 'capitalize', 'strip_blank_lists', 'split_on_blanks', 'remove_duplicate_names', 'remove_bad_rows', 'columnize', ] def do_scrub(self): """Runs all functions in self.functions that weren't negated by command- line arguments. Writes a clean file and, optionally, a file of rows that couldn't be properly scrubbed """ self.fxns = [x for x in self.functions if x not in self.flags] self.grab_file_data(self.in_file) for fx in self.fxns: next_operation = getattr(self, fx) next_operation() if self.manual_repair: self.write_csv(self.manual_repair, self.dirty_out_file) self.write_csv(self.rows, self.clean_out_file) def grab_file_data(self, filename): """Opens the file passed as filename and writes rows to a list of lists. """ with open(filename, 'rt') as opened_file: read_file = csv.reader(opened_file) for row in read_file: # [q1] self.rows.append(row) opened_file.close def strip_blank_fields(self): """If there are any blank fields in a row, take them output. """ for row in self.rows: while "" in row: row.remove("") def strip_whitespace(self): """If there is whitespace in a field, take it out.""" for row in self.rows: for num, field in enumerate(row): row[num] = field.strip() def capitalize(self): """Make all non email fields capitalized (string.title()). """ for row in self.rows: for num, field in enumerate(row): if "@" not in field and not field.istitle(): row[num] = field.title() def strip_blank_lists(self): """Remove any rows that are blank. """ while  in self.rows: self.rows.remove() def split_on_blanks(self): """For fields that have a space between two words, split them out into seperate fields """ for row in self.rows: for num, field in enumerate(row): if ' ' in field: x = field.split(' ') row.pop(num) for i in x: row.append(i) # Move all the emails addresses to the last column. for row in self.rows: for num, field in enumerate(row): if "@" in field: x = row.pop(num) row.append(x) def remove_duplicate_names(self): """If two columns have the same name in them, remove one of the names. """ for num, row in (enumerate(self.rows)): for x in xrange(len(row)): if row.count(row[x]) > 1: row.pop(x) break def remove_bad_rows(self): """Remove all rows that don't have at least three fields filled. Assumes that rows with less than three fields means either missing first, last, or email. """ for num, row in enumerate(self.rows): if len(row) < 3: x = self.rows.pop(num) self.manual_repair.append(x) # Remove all rows that don't have an email address. [q2] for num, row in enumerate(self.rows): bad = True for field in row: if '@' in field: bad = False break if bad == True: x = self.rows.pop(num) self.manual_repair.append(x) def columnize(self): """If there's no title (Mr, Mrs, etc), put a space in the first column. """ titles = [ 'Mr', 'Mrs', 'Mr.', 'Mrs.', 'mr', 'mrs', 'mr.', 'mrs.', 'Miss', 'miss' ] for row in self.rows: if set(row).isdisjoint(set(titles)): row.insert(0, '') def write_csv(self, rows, name_to_write): """Writes a csv based on a list of lists as data for the rows, and a name of the file to write (string). """ f = open(name_to_write, 'wt') try: writer = csv.writer(f) writer.writerow(('Title', 'First', 'Middle', 'Last', 'Email')) for row in rows: writer.writerow(row) finally: f.close() if __name__ == "__main__": x = CleanCsv(sys.argv) x.do_scrub()
- Is this well suited as an OOP? It makes it easier to organize, but what are the downsides of using OOP in this case? Should I separate the worker functions into a different class from the engine/control functions?
- Am I missing any PEP8 stuff?
- There's a bunch of other classes and functions in the csv module. Is there anything I could have used?
- Is there any reason to split this into two different files?
- What are the components of the Python language that I'm missing? What features of the standard library could I use to make this better?
- Is there too much nesting? My general sense is that less nesting is better for clarity, but I haven't seen a way to get around it here.
Specific In-Line Questions
[q1] I feel like I should be able to access this without having to re-write it to lists?
[q2] Maybe this loop should be moved into another function for modularity?