4
\$\begingroup\$

The following is a python program processing files input and generating a .csv output. There are two input files of .txt format, whose main body are of the format as follows:

video_name method_name
criteria_1_result(Named 'AUC' criteria)
criteria_2_result(Named 'DP' criteria)

And two final average result lines and a may-not-exist EOF indicator(>>) are at the end.

And what I want to do is to generate a .csv file to compare results of the two criteria for different method and different videos. And I used a dict whose key is the video sequence name and whose value is another dict containing what I what to save to the .csv file.

Any suggestions to improve this small program?

Sample input:

1.txt:

video1 method1
10.0
11.0
video2 method1
20.0
9.0
15.0
10.0
>>

2.txt

video3 method2
10.0
11.0
video2 method2
20.0
9.0
video1 method2
15.0
10.0
15.0
10.0

Sample output:

"Seq","AUC_1","AUC_2","AUC_Diff","DP_1","DP_2","DP_Diff"
"video1","10.0","15.0","5.0","11.0","10.0","1.0"
"video2","20.0","20.0","0.0","9.0","9.0","0.0"
"video3","-","10.0","-","-","11.0","-"

My code:

# -*- coding: utf-8 -*-
# Created At 2017-09-18 10:34
# Version: 1.0
# License: GPL 2.0
# Description: 
# This program read two input files, saving data and difference 
# to a csv file for comparision 


import csv
import math


def read_file(f_name):
    """
    Read and get file content,
    remove redundant '>>' at the end if exists
    :param f_name: Name of the file to read
    :return: List of lines of the file
    """
    with open(f_name, 'r') as f:
        content = f.readlines()
        if content[-1].strip() == ">>":
            content = content[:-1]
    return content


def fill_record(f_content, record, csv_headers, idf):
    """
    Using content read from input file, fill 'record' dict.
    Dict keys are specified from 'csv_headers'.
    'idf' indicates the suffix of columns like '1' in 'AUC_1'
    :param f_content: list of input file lines
    :param record: dict for saving data
    :param csv_headers: list specifying dict keys
    :param idf: '1' or '2', indicating the suffix like '1' in 'AUC_1'
    :return: return dict filled with data from both input and calculated
    """
    it_f = 0
    while it_f < len(f_content)-2:
        seq_name = f_content[it_f].strip().split(' ')[0].lower()
        if seq_name not in record:
            record[seq_name] = {}
            for column in csv_headers:
                record[seq_name][column] = '-'
        record[seq_name]['AUC_'+idf] = f_content[it_f+1].strip()
        record[seq_name]['DP_'+idf] = f_content[it_f+2].strip()
        if record[seq_name]['AUC_1'] != '-' and record[seq_name]['AUC_2'] != '-':
            record[seq_name]['AUC_Diff'] = str(math.fabs(float(record[seq_name]['AUC_1']) -
                                                         float(record[seq_name]['AUC_2'])))
            record[seq_name]['DP_Diff'] = str(math.fabs(float(record[seq_name]['DP_1']) -
                                                        float(record[seq_name]['DP_2'])))
        it_f += 3 
    return record


def write_to_csv_file(record, csv_headers, compare_result_name):
    """
    Writing data to .csv files.
    :param record: Dict filled with data to write
    :param csv_headers: Dict keys / CSV file columns
    :param compare_result_name: file name for output .csv file
    :return: None
    """
    with open(compare_result_name, 'w') as csvf:
        writer = csv.writer(csvf, quoting=csv.QUOTE_ALL)
        writer.writerow(csv_headers)
        for seq in record:
            row = ['-' for i in range(7)]
            row[0] = seq
            for i in range(len(csv_headers[1:])):
                row[1+i] = record[seq][csv_headers[1+i]]
            writer.writerow(row)


def main():
    """
    Main function
    :return: None
    """
    # Change the input file names and output file name in need
    f1_name = '1.txt'
    f2_name = '2.txt'
    compare_result_name = 'compare.csv'
    csv_headers = ['Seq', 'AUC_1', 'AUC_2', 'AUC_Diff', 'DP_1', 'DP_2', 'DP_Diff']
    record = {}

    f1_content = read_file(f1_name)
    f2_content = read_file(f2_name)
    record = fill_record(f1_content, record, csv_headers, '1')
    record = fill_record(f2_content, record, csv_headers, '2')
    write_to_csv_file(record, csv_headers, compare_result_name)


if __name__ == "__main__":
    main()
\$\endgroup\$
4
\$\begingroup\$

Thank you for the docstrings; these are great. There is very little redundancy in your programme.

I have a few suggestions, however:

  1. Switch math.fabs for abs: math.fabs is great for dealing with numeric arguments that aren't floats. However, given that you're doing the float conversion anyway, you could use abs to the same effect. This also saves you from having to import the math package.

Before:

    record[seq_name]['AUC_Diff'] = str(math.fabs(float(record[seq_name]['AUC_1']) -
                                       float(record[seq_name]['AUC_2'])))

After:

    record[seq_name]['AUC_Diff'] = str(abs(float(record[seq_name]['AUC_1']) - 
                                       float(record[seq_name]['AUC_2'])))
  1. Use defaultdict to define default data: You pre-populate the record with '-' when you cycle through the files to help identify when a value is missing. One neat trick would be to use a defaultdict, which nicely defines the default values of missing data.

Before (in body of fill_record; this can be removed):

    if seq_name not in record:
        record[seq_name] = {}
        for column in csv_headers:
            record[seq_name][column] = '-'

After (in main; this is inserted in place of the definition of record):

    from collections import defaultdict
    d = lambda: {column: '-' for column in csv_headers}
    record = defaultdict(d)
  1. Use list slicing to cycle through file contents: In fill_record, you can cycle through each 3rd row in one line (rather than using while and the it_f counter) using list slicing:

Before:

    it_f = 0
    while it_f < len(f_content) - 2:
        seq_name = f_content[it_f].strip().split(' ')[0].lower()
        ...
        it_f += 3

After:

    for i, header in enumerate(f_content[:-2:3]):
        seq_name = header.strip().split(' ')[0].lower()
        it_f = i * 3
        ...
  1. Simplify writing files: you can simplify writing to csv as follows:

Before:

    with open(compare_result_name, 'w') as csvf:
    writer = csv.writer(csvf, quoting=csv.QUOTE_ALL)
    writer.writerow(csv_headers)
    for seq in record:
        row = ['-' for i in range(7)]
        row[0] = seq
        for i in range(len(csv_headers[1:])):
            row[1+i] = record[seq][csv_headers[1+i]]
        writer.writerow(row)

After:

    with open(compare_result_name, 'w') as csvf:
        writer = csv.writer(csvf, quoting=csv.QUOTE_ALL)
        writer.writerow(csv_headers)
        for seq, row in record.items():
            writer.writerow([seq] + [row[c] for c in csv_headers[1:]])

Usually, it would be tempting to make suggestions about how one could make the functionality of a programme more flexible or extensible so that you could build out extra functionality in the future, or port functions to other programmes. I get the impression that this wouldn't be so useful in this case given the fixed formats of the inputs. Is this right? Are you expecting to use or adapt this programme in the future?

\$\endgroup\$
  • \$\begingroup\$ Yes. Since the format here is fixed, I mainly just want to know where to imporve my program from the code style view and not so much form the robustness view. Thanks for your kind and detailed answer. And your point 1, 3 and 5 are quite useful for me. Point 2 is not so perfect for readlines() return a list and cannot be chained to rstrip(). And for point 4, range() doesn't accept a keyword parameter(py version issue? py3 for me) \$\endgroup\$ – lincr Sep 20 '17 at 3:27
  • \$\begingroup\$ Hi lincr. Sorry for the mistakes there. I have edited the answer accordingly. On range(): this was my mistake (I was trying to be clever while afk); I included the zip(range(...), ...) component as list slicing returns a copy of a list (rather than an iterator that preserves the indexes of the original list). Without it, the indexes in my new for loop count: 0, 1, 2, ... rather than 0, 3, 6, ... (which is the desired behaviour). Instead, I have added the line it_f = i * 3. \$\endgroup\$ – act Sep 20 '17 at 8:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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