Python-将CSV转换为对象-代码设计 [英] Python - Converting CSV to Objects - Code Design

查看:59
本文介绍了Python-将CSV转换为对象-代码设计的处理方法,对大家解决问题具有一定的参考价值,需要的朋友们下面随着小编来一起学习吧!

问题描述

我有一个小脚本,用于读取包含员工的CSV文件,并对这些数据执行一些基本操作.

我们读入数据(import_gd_dump),并创建一个Employees对象,其中包含Employee对象的列表(也许我应该想到一个更好的命名约定...大声笑).然后,我们在Employees上调用clean_all_phone_numbers(),在每个Employee上分别调用clean_phone_number(),在Employees上分别调用lookup_all_supervisors().

import csv
import re
import sys

#class CSVLoader:
#    """Virtual class to assist with loading in CSV files."""
#    def import_gd_dump(self, input_file='Gp Directory 20100331 original.csv'):
#        gd_extract = csv.DictReader(open(input_file), dialect='excel')
#        employees = []
#        for row in gd_extract:
#            curr_employee = Employee(row)
#            employees.append(curr_employee)
#        return employees
#    #self.employees = {row['dbdirid']:row for row in gd_extract}

# Previously, this was inside a (virtual) class called "CSVLoader".
# However, according to here (http://tomayko.com/writings/the-static-method-thing) - the idiomatic way of doing this in Python is not with a class-function but with a module-level function
def import_gd_dump(input_file='Gp Directory 20100331 original.csv'):
    """Return a list ('employee') of dict objects, taken from a Group Directory CSV file."""
    gd_extract = csv.DictReader(open(input_file), dialect='excel')
    employees = []
    for row in gd_extract:
        employees.append(row)
    return employees

def write_gd_formatted(employees_dict, output_file="gd_formatted.csv"):
    """Read in an Employees() object, and write out each Employee() inside this to a CSV file"""
    gd_output_fieldnames = ('hrid', 'mail', 'givenName', 'sn', 'dbcostcenter', 'dbdirid', 'hrreportsto', 'PHFull', 'PHFull_message', 'SupervisorEmail', 'SupervisorFirstName', 'SupervisorSurname')
    try:
        gd_formatted = csv.DictWriter(open(output_file, 'w', newline=''), fieldnames=gd_output_fieldnames, extrasaction='ignore', dialect='excel')
    except IOError:
        print('Unable to open file, IO error (Is it locked?)')
        sys.exit(1)

    headers = {n:n for n in gd_output_fieldnames}
    gd_formatted.writerow(headers)
    for employee in employees_dict.employee_list:
        # We're using the employee object's inbuilt __dict__ attribute - hmm, is this good practice?
        gd_formatted.writerow(employee.__dict__)

class Employee:
    """An Employee in the system, with employee attributes (name, email, cost-centre etc.)"""
    def __init__(self, employee_attributes):
        """We use the Employee constructor to convert a dictionary into instance attributes."""
        for k, v in employee_attributes.items():
            setattr(self, k, v)

    def clean_phone_number(self):
        """Perform some rudimentary checks and corrections, to make sure numbers are in the right format.
        Numbers should be in the form 0XYYYYYYYY, where X is the area code, and Y is the local number."""
        if self.telephoneNumber is None or self.telephoneNumber == '':
            return '', 'Missing phone number.'
        else:
            standard_format = re.compile(r'^\+(?P<intl_prefix>\d{2})\((?P<area_code>\d)\)(?P<local_first_half>\d{4})-(?P<local_second_half>\d{4})')
            extra_zero = re.compile(r'^\+(?P<intl_prefix>\d{2})\(0(?P<area_code>\d)\)(?P<local_first_half>\d{4})-(?P<local_second_half>\d{4})')
            missing_hyphen = re.compile(r'^\+(?P<intl_prefix>\d{2})\(0(?P<area_code>\d)\)(?P<local_first_half>\d{4})(?P<local_second_half>\d{4})')
            if standard_format.search(self.telephoneNumber):
                result = standard_format.search(self.telephoneNumber)
                return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), ''
            elif extra_zero.search(self.telephoneNumber):
                result = extra_zero.search(self.telephoneNumber)
                return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), 'Extra zero in area code - ask user to remediate. '
            elif missing_hyphen.search(self.telephoneNumber):
                result = missing_hyphen.search(self.telephoneNumber)
                return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), 'Missing hyphen in local component - ask user to remediate. '
            else:
                return '', "Number didn't match recognised format. Original text is: " + self.telephoneNumber

class Employees:
    def __init__(self, import_list):
        self.employee_list = []    
        for employee in import_list:
            self.employee_list.append(Employee(employee))

    def clean_all_phone_numbers(self):
        for employee in self.employee_list:
            #Should we just set this directly in Employee.clean_phone_number() instead?
            employee.PHFull, employee.PHFull_message = employee.clean_phone_number()

    # Hmm, the search is O(n^2) - there's probably a better way of doing this search?
    def lookup_all_supervisors(self):
        for employee in self.employee_list:
            if employee.hrreportsto is not None and employee.hrreportsto != '':
                for supervisor in self.employee_list:
                    if supervisor.hrid == employee.hrreportsto:
                        (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = supervisor.mail, supervisor.givenName, supervisor.sn
                        break
                else:
                    (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = ('Supervisor not found.', 'Supervisor not found.', 'Supervisor not found.')
            else:
                (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = ('Supervisor not set.', 'Supervisor not set.', 'Supervisor not set.')

    #Is thre a more pythonic way of doing this?
    def print_employees(self):
        for employee in self.employee_list:
            print(employee.__dict__)

if __name__ == '__main__':
    db_employees = Employees(import_gd_dump())
    db_employees.clean_all_phone_numbers()
    db_employees.lookup_all_supervisors()
    #db_employees.print_employees()
    write_gd_formatted(db_employees)

首先,我的序言问题是,从类设计或Python的角度来看,您能看到上述内容固有的问题吗?逻辑/设计合理吗?

无论如何,具体来说:

  1. Employees对象具有方法clean_all_phone_numbers(),该方法对其中的每个Employee对象调用clean_phone_number().这是不好的设计吗?如果是这样,为什么?另外,我叫lookup_all_supervisors()的方式不好吗?
  2. 最初,我将clean_phone_number()lookup_supervisor()方法包装在一个函数中,并在其中包含一个for循环. clean_phone_number是O(n),我相信lookup_supervisor是O(n ^ 2)-可以将它分成两个这样的循环吗?
  3. clean_all_phone_numbers()中,我在Employee对象上循环,并使用return/assignment设置它们的值-我应该在clean_phone_number()本身中设置它吗?

还有一些我被黑客破解的东西,不确定它们是否是不好的做法-例如print_employee()gd_formatted()都使用__dict__,而Employee的构造函数使用setattr()将字典转换为实例属性.

我将完全重视任何想法.如果您认为问题过于笼统,请告诉我,我可以将其拆分成几个部分重新发布(我只是不想用多个类似的问题来污染董事会,而三个问题或多或少地紧密相关). /p>

干杯, 维克多

解决方案

对我来说还不错.做得好.您打算多久运行一次此脚本?如果这是一次性的事情,那么您的大多数问题都没有道理.

  1. 我喜欢Employees.cleen_all_phone_numbers()委派给Employee.clean_phone_number()
  2. 的方式
  3. 您确实应该在此处使用索引(词典).您可以在O(n)中创建每个雇员时按hrid为其编制索引,然后在O(1)中对其进行查找.
    • 但是只有在必须再次运行脚本的情况下才这样做...
    • 只需养成使用字典的习惯.它们很轻松,并且使代码更易于阅读.每当您编写方法lookup_*时,您可能只想索引字典.
  4. 不确定.我喜欢明确设置状态,但这实际上是一个糟糕的设计-clean_phone_number()应该这样做,员工应该对自己的状态负责.

I have a small script we're using to read in a CSV file containing employees, and perform some basic manipulations on that data.

We read in the data (import_gd_dump), and create an Employees object, containing a list of Employee objects (maybe I should think of a better naming convention...lol). We then call clean_all_phone_numbers() on Employees, which calls clean_phone_number() on each Employee, as well as lookup_all_supervisors(), on Employees.

import csv
import re
import sys

#class CSVLoader:
#    """Virtual class to assist with loading in CSV files."""
#    def import_gd_dump(self, input_file='Gp Directory 20100331 original.csv'):
#        gd_extract = csv.DictReader(open(input_file), dialect='excel')
#        employees = []
#        for row in gd_extract:
#            curr_employee = Employee(row)
#            employees.append(curr_employee)
#        return employees
#    #self.employees = {row['dbdirid']:row for row in gd_extract}

# Previously, this was inside a (virtual) class called "CSVLoader".
# However, according to here (http://tomayko.com/writings/the-static-method-thing) - the idiomatic way of doing this in Python is not with a class-function but with a module-level function
def import_gd_dump(input_file='Gp Directory 20100331 original.csv'):
    """Return a list ('employee') of dict objects, taken from a Group Directory CSV file."""
    gd_extract = csv.DictReader(open(input_file), dialect='excel')
    employees = []
    for row in gd_extract:
        employees.append(row)
    return employees

def write_gd_formatted(employees_dict, output_file="gd_formatted.csv"):
    """Read in an Employees() object, and write out each Employee() inside this to a CSV file"""
    gd_output_fieldnames = ('hrid', 'mail', 'givenName', 'sn', 'dbcostcenter', 'dbdirid', 'hrreportsto', 'PHFull', 'PHFull_message', 'SupervisorEmail', 'SupervisorFirstName', 'SupervisorSurname')
    try:
        gd_formatted = csv.DictWriter(open(output_file, 'w', newline=''), fieldnames=gd_output_fieldnames, extrasaction='ignore', dialect='excel')
    except IOError:
        print('Unable to open file, IO error (Is it locked?)')
        sys.exit(1)

    headers = {n:n for n in gd_output_fieldnames}
    gd_formatted.writerow(headers)
    for employee in employees_dict.employee_list:
        # We're using the employee object's inbuilt __dict__ attribute - hmm, is this good practice?
        gd_formatted.writerow(employee.__dict__)

class Employee:
    """An Employee in the system, with employee attributes (name, email, cost-centre etc.)"""
    def __init__(self, employee_attributes):
        """We use the Employee constructor to convert a dictionary into instance attributes."""
        for k, v in employee_attributes.items():
            setattr(self, k, v)

    def clean_phone_number(self):
        """Perform some rudimentary checks and corrections, to make sure numbers are in the right format.
        Numbers should be in the form 0XYYYYYYYY, where X is the area code, and Y is the local number."""
        if self.telephoneNumber is None or self.telephoneNumber == '':
            return '', 'Missing phone number.'
        else:
            standard_format = re.compile(r'^\+(?P<intl_prefix>\d{2})\((?P<area_code>\d)\)(?P<local_first_half>\d{4})-(?P<local_second_half>\d{4})')
            extra_zero = re.compile(r'^\+(?P<intl_prefix>\d{2})\(0(?P<area_code>\d)\)(?P<local_first_half>\d{4})-(?P<local_second_half>\d{4})')
            missing_hyphen = re.compile(r'^\+(?P<intl_prefix>\d{2})\(0(?P<area_code>\d)\)(?P<local_first_half>\d{4})(?P<local_second_half>\d{4})')
            if standard_format.search(self.telephoneNumber):
                result = standard_format.search(self.telephoneNumber)
                return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), ''
            elif extra_zero.search(self.telephoneNumber):
                result = extra_zero.search(self.telephoneNumber)
                return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), 'Extra zero in area code - ask user to remediate. '
            elif missing_hyphen.search(self.telephoneNumber):
                result = missing_hyphen.search(self.telephoneNumber)
                return '0' + result.group('area_code') + result.group('local_first_half') + result.group('local_second_half'), 'Missing hyphen in local component - ask user to remediate. '
            else:
                return '', "Number didn't match recognised format. Original text is: " + self.telephoneNumber

class Employees:
    def __init__(self, import_list):
        self.employee_list = []    
        for employee in import_list:
            self.employee_list.append(Employee(employee))

    def clean_all_phone_numbers(self):
        for employee in self.employee_list:
            #Should we just set this directly in Employee.clean_phone_number() instead?
            employee.PHFull, employee.PHFull_message = employee.clean_phone_number()

    # Hmm, the search is O(n^2) - there's probably a better way of doing this search?
    def lookup_all_supervisors(self):
        for employee in self.employee_list:
            if employee.hrreportsto is not None and employee.hrreportsto != '':
                for supervisor in self.employee_list:
                    if supervisor.hrid == employee.hrreportsto:
                        (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = supervisor.mail, supervisor.givenName, supervisor.sn
                        break
                else:
                    (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = ('Supervisor not found.', 'Supervisor not found.', 'Supervisor not found.')
            else:
                (employee.SupervisorEmail, employee.SupervisorFirstName, employee.SupervisorSurname) = ('Supervisor not set.', 'Supervisor not set.', 'Supervisor not set.')

    #Is thre a more pythonic way of doing this?
    def print_employees(self):
        for employee in self.employee_list:
            print(employee.__dict__)

if __name__ == '__main__':
    db_employees = Employees(import_gd_dump())
    db_employees.clean_all_phone_numbers()
    db_employees.lookup_all_supervisors()
    #db_employees.print_employees()
    write_gd_formatted(db_employees)

Firstly, my preamble question is, can you see anything inherently wrong with the above, from either a class design or Python point-of-view? Is the logic/design sound?

Anyhow, to the specifics:

  1. The Employees object has a method, clean_all_phone_numbers(), which calls clean_phone_number() on each Employee object inside it. Is this bad design? If so, why? Also, is the way I'm calling lookup_all_supervisors() bad?
  2. Originally, I wrapped the clean_phone_number() and lookup_supervisor() method in a single function, with a single for-loop inside it. clean_phone_number is O(n), I believe, lookup_supervisor is O(n^2) - is it ok splitting it into two loops like this?
  3. In clean_all_phone_numbers(), I'm looping on the Employee objects, and settings their values using return/assignment - should I be setting this inside clean_phone_number() itself?

There's also a few things that I'm sorted of hacked out, not sure if they're bad practice - e.g. print_employee() and gd_formatted() both use __dict__, and the constructor for Employee uses setattr() to convert a dictionary into instance attributes.

I'd value any thoughts at all. If you think the questions are too broad, let me know and I can repost as several split up (I just didn't want to pollute the boards with multiple similar questions, and the three questions are more or less fairly tightly related).

Cheers, Victor

解决方案

Looks fine to me. Good job. How often are you going to run this script? Most of your questions are moot if this is a one-off thing.

  1. I like the way Employees.cleen_all_phone_numbers() delegates to Employee.clean_phone_number()
  2. You really should be using an index (dictionary) here. You can index each employee by hrid when you create them in O(n) and then look them up in O(1).
    • But only do this if you ever have to run the script again...
    • Just get into the habit of using dictionaries. They are painless and make code easier to read. Whenever you write a method lookup_* you probably just want to index a dictionary.
  3. not sure. I like explicitly setting state, but this is actually bad design - clean_phone_number() should do that, Employees should be responsible for their own state.

这篇关于Python-将CSV转换为对象-代码设计的文章就介绍到这了,希望我们推荐的答案对大家有所帮助,也希望大家多多支持IT屋!

查看全文
登录 关闭
扫码关注1秒登录
发送“验证码”获取 | 15天全站免登陆