7
\$\begingroup\$

I'm fairly new to python and pandas but trying to get better with it for parsing and processing large data files. I'm currently working on a project that requires me to parse a a few hundred CSV CAN files at the time. The files have 9 columns of interest (1 ID and 7 data fields), have about 1-2 million rows, and are encoded in hex.

  id   Flags   DLC Data0   Data1   Data2   Data3   Data4   Data5   Data6   Data7
cf11505  4      1   ff                          
cf11505  4      1   ff                          
cf11505  4      1   ff                          
cf11a05  4      1   0                           
cf11505  4      1   ff                          
cf11505  4      1   ff                          
cf11505  4      1   ff                          
cf11005  4      8   ff       ff      ff      ff      ff       ff      ff     ff

I need to decode the hex, and then extract a bunch of different variables from it depending on the CAN ID.

I wrote a script to parse these files that looks like this:

import os
import csv     # imports the csv module
import itertools
import datetime
import time
from tkinter import filedialog
from tkinter import Tk

Tk().withdraw()
filenames = filedialog.askopenfiles(title="Select .csv log file", filetypes=(("CSV files", "*.csv"), ("all files", "*.*")))

start = time.clock()

flist = []
for name in filenames: 

    flist.append(name.name)

for filename in flist: 
    cur_file = ''.join(filename.split('/')[-1:])
    print('working on ' + cur_file +'...')

    time_s = 0
    var = [0,0,0,0]
    var4 = 0
    var5 = 0
    var6 = 0
    var7 = 0
    var8 = 0
    var9 = 0
    var10 = 0
    var11 = 0
    var12 = 0

    ###########create new filename and filepath
    cur_file = ''.join(filename.split('/')[-1:]) #pulls filename of current file

    folders_to_append = '/Log Files--Processed/' + '/'.join(flist[0].split('/')[-3:-1]) #folders to append to new filepath
    trunc_filepath = '/'.join(filename.split('/')[0:-4]) 

    new_filepath = trunc_filepath + folders_to_append + '/'
    new_filename= trunc_filepath + folders_to_append + '/processed_' + cur_file

    if not os.path.exists(new_filepath):
        os.makedirs(new_filepath)
    ##########################################

    csvInput = open(filename, 'r') # opens the csv file
    csvOutput = open(new_filename, 'w', newline='')

    writer = csv.writer(csvOutput) #creates the writer object
    writer.writerow(['Date','Time Since Start (s)', 'var1', 'var2', 'var3', 'var4', 'var5', 'var6', 
                     'var7', 'var8', 'var9', 'var10', 'var11', 'var12', 'var13', 'var14', 'var15', 'var16'])

    try:
        reader = csv.reader(csvInput)
        data=list(reader)

        if (data[3][1] == 'HEX'): dataType = 16
        elif (data[3][1] == 'DEC'): dataType = 10
        else: print('Invalid Data Type')

        if (data[4][1] == 'HEX'): idType = 16
        elif (data[4][1] == 'DEC'): idType = 10
        else: print('Invalid ID Type') 

        start_date = datetime.datetime.strptime(data[6][1],'%Y-%m-%d %H:%M:%S')      

        for row in itertools.islice(data,8,None):
            #print(row) 
            try: ID = int(row[2],idType)
            except: ID = 0

            #print(ID)

            if (ID == 0xcf11005):
                for i in range(0,4): var[i] = float((int(row[2*i+6],dataType)<<8)|(int(row[2*i+5],dataType)))/10
            elif (ID == 0xcf11505):
                var4 = int(row[5],dataType)
            elif (ID == 0xcf11605):
                var8 = str(bin(int(row[5],dataType)))
                var9 = str(bin(int(row[6],dataType)));
            elif (ID == 0xcf11a05):
                var10 = row[5]
            elif (ID == 0xcf11e05):
                var11 = float((int(row[6],dataType)<<8)|(int(row[5],dataType)))
                var12 = float((int(row[8],dataType)<<8)|(int(row[7],dataType)))
                var13 = float((int(row[10],dataType)<<8)|(int(row[9],dataType)))
            elif (ID == 0xcf11f05):
                var14 = int(row[5],dataType)
                var15 = int(row[6],dataType)-40
                var16 = int(row[7],dataType)-40
            else:
                continue

            time_s = float(row[0])
            date = start_date+datetime.timedelta(seconds=time_s)
            writer.writerow([date,time_s, var[0], var[1], var[2], var[3], var4, var5, var6, var7, var8, var9, var10, var11, var12])


    finally:
            csvInput.close()
            csvOutput.close()

end = time.clock()

print(end - start)
print('done')

It basically uses the CSV reader and writer to generate a processed CSV file line by line for each CSV. For a 2 million row CSV CAN file, it takes about 40 secs to fully run on my work desktop. Knowing that line by line iteration is much slower than performing vectorized operations on a pandas dataframe, I thought I could do better, so I wrote a separate script (which I'll call script #2) where all the math was performed in a vectorized fashion, and then I used pandas .to_csv() function. Unfortunately, with script #2, the .to_csv() function took as long to run as the entire script #1, so it didn't end up being faster. I played around with the chunk size of .to_csv(), but I never managed to improve the runtime more than a second or so.

Is using the CSV reader/writer really the fastest way to do this? And if so, is there any way I can make my code run faster?

\$\endgroup\$
4
\$\begingroup\$

I will avoid using pandas for now, since it can be really easy to say "pandas will make X faster" and miss out on making what is currently written performant, even though it may be true. Someone else can show the light of numpy/pandas as it pertains to this example.

Style

Variable Naming

Variables and functions should be lowercased and snake-case, some_variable or var. Objects such as classes should have names like SomeClass. Also, names like var, var1, var666 should usually be swapped out for names a bit more meaningful to make it easier to deduce what values mean. They are names, after all. This depends on your use case, and your columns have those names as well, so maybe it makes sense to you. I'll keep them as-is (except for the snake_case change) for the sake of your example.

Spacing Between Operators

Make sure you have whitespace between operators and commas:

# from this
val = (a,b)
val2 = a+b

# to this
val = (a, b)
val2 = a + b

One-line if and for

It makes code more difficult to read with lots of statements such as:

if condition: var = 1
else: var = 23

if other_condition: var2 = 1
else: var2 = 33

for item in iterable: # long expression here

Breaking these up into multiple lines makes it much easier to visually scan through for variables, operations, etc, since the eye only has to keep track of the indentation level:

if condition:
    var = 1
else:
    var = 2


for item in iterable:
    # long expression here

String concatenation

There are a lot of instances of str + str2. I'd suggest using either str.format or f-strings if you are in python 3.5+:

# str.format
a, b = 'hello', 'world' 
some_string = '{0}, {1}!'.format(a, b)
print(some_string)
hello, world!

some_path = '{0}/{1}/file.txt'.format(a, b)
print(some_path)
hello/world/file.txt

# f-strings
some_string = f'{a}, {b}!'
print(some_string)
hello, world!

some_path = f'{a}/{b}/file.txt'
print(some_path)
hello/world/file.txt

Functions

It can be helpful to organize code into functions, rather than running everything in one go. This makes code easier to maintain and update in pieces. I'll go into more specifics later

Exceptions

It's generally bad practice to generically handle exceptions like:

try:
    something
except:
    pass

# or
try:
    something
except Exception:
    pass

You should explicitly handle expected errors, and raise unexpected ones. These blocks should also generally surround as little code as possible, ideally only what will raise an exception. Otherwise, it can be a nightmare to debug.

Re-factoring

Last value of a list

Statements such as some_str.split('/')[-1:] don't need the trailing :, since you are just grabbing the last element. This eliminates the need to join, since you aren't slicing a list, instead just grabbing the last element which is a string:

# this
cur_file = ''.join(filename.split('/')[-1:])

# should be this
cur_file = filename.split('/')[-1]

You actually do this twice, so the second one can be removed.

Creating folders

Part of the directory is a constant, as denoted by the hard-coded string and the flist[0].split(...)... line. This can be taken out of the for loop, then you just need to join that with the filename slice. Also, it's usually not a great idea to put spaces in file/folder names, so I'd avoid it:

import os

# use os.path.join here
folders = os.path.join( 
    'log_files_processed', 
    *flist[0].split('/')[-3:-1] # this unpacks the result of the slice
)

for filename in flist:
    # you don't need the leading zero in the slice
    # and join with the other string you created
    to_create = os.path.join(*filename.split('/')[:-4], folders)

    # you can use exist_ok=True here to avoid the if exists check
    os.makedirs(to_create, exist_ok=True)
    ...

bin(int)

No need to do str(bin(some_value)), since bin returns a string already

int to byte compare

0xa == int('10', 16)
False

int(0xa)
10

I'd look into your core logic here and finding a different way to check against a bytes value. You might consider using string formatting (modified from this answer)

int(0xcf11505)
217126149

f'{format(217126149, "#02x")}'
'0xcf11505'

# an empty string for the second argument keeps it in decimal format
f'{format(217126149, "")}'
'217126149'

Invalid Types

You denote invalid types, however, instead of skipping the file, you continue, causing a NameError when ID is not defined:

# this
if data[3][1] == 'HEX':
    id_type = 16
elif data[3][1] == 'DEC':
    id_type = 10
else:
    print('Invalid Type')

# should be this
if data[3][1] == 'HEX':
    data_type = 16
elif data[3][1] == 'DEC':
    data_type = 10
else:
    print('Invalid Type')
    # this will go to the next iteration over flist
    # after refactoring the loop
    continue 

Iterating over reader

The biggest place you are losing speed is by not directly iterating over the csv.reader. The reason being is that you do list(reader), which iterates over the reader once and pulls everything into memory, then you pass it to itertools to iterate over it again. Your use of islice is good, and helps you to a degree, but it doesn't gain back everything lost from list(reader). To show what this does, I've created a 500000 line, 4 column csv file to just iterate over with random integers:

import csv
from itertools import islice

# testing this method first
def list_method():
    with open('random.csv') as fh:
        reader = csv.reader(fh)
        data = list(reader)

        x = data[7][1]

        for line in islice(data, 8, None):
            # just reading, not adding anything else to it
            line = line


# then this method
def read_method():
    with open('random.csv') as fh:
        reader = csv.reader(fh)

        x = next(next(reader) for i in range(8) if i == 7)[1]

        for line in reader:
            line = line

Results:

python -m timeit -s 'from something import list_method, read_method' 'list_method()'
1 loop, best of 5: 225 msec per loop

python -m timeit -s 'from something import list_method, read_method' 'read_method()'
2 loops, best of 5: 148 msec per loop

So holding all else constant, you get a 34% boost just in the basic loop.

for filename in flist:
    cur_file = filename.split('/')[-1]
    # use f-string here for readability
    print(f'working on {cur_file}...')

    ~skipping lots of other code~

    with open(filename) as csv_in, with open(new_filename, 'w') as csv_out:
        reader = csv.reader(csv_in)
        writer = csv.writer(csv_out)

        # grab the first 8 lines, this also consumes from reader
        # so you don't have to use itertools.islice
        data = [next(reader) for i in range(7)]

        types = {'HEX': '#02x', 'DEC': ''}

        # this cleans up the check to data_type and id_type
        try:
            data_str, id_str = types[data[3][1]], types[data[4][1]]
        except KeyError:
            print('Got invalid type')
            continue

        # iterate directly over the reader, then you don't have to
        # worry about collecting everything into a list
        for row in reader:
            _id = f'{format(row[2], id_str)}'
            # rest of iteration

Note the use of with to open both files. I've renamed them csv_in and csv_out. Calling next(reader) will consume lines from reader. The benefit here is that reader acts like a generator in that the next time you try to iterate over reader, it will start where you left off. In this case, line 8, eliminating the need for itertools.islice. Then, the only thing aggregated into memory are the first 8 lines for value checking.

Next, we can use a dictionary to eliminate the redundant if val == 'HEX': 16; elif... blocks. Two things worth noting

  1. I'm returning the strings in order to format the values into either hex or decimal formats as I showed in one of the suggested edits above

  2. I'm using direct access on that dictionary because an invalid key will raise a KeyError, which I can catch to skip that file rather than waiting for a giant try/except to catch the NameError you will otherwise generate. As a small example:

x = 5

if x == 4:
    y = 3
elif x == 6:
    y = 2:
else:
    # this branch never defines y, so
    # a NameError will occur if I try to
    # access it later
    print('Invalid!')
Invalid!

print(y)
NameError: y is not defined

It might be good to refactor some of the above code into a function for two reasons: 1. I don't want to lug the types dictionary and data list around 2. The KeyError can just be caught around the function, making the loop easier to read

def data_and_id_type(reader):
    """
    takes a csv.reader instance to iterate over. Will match 
    data_type and id_type to either 'HEX' or 'DEC' and return 
    tuples of the base to be passed to int and the format
    string. If neither match, a KeyError will be raised
    """
    types = {'DEC': (10, ''), 'HEX': (16, '#02x')}

    rows = [next(reader) for i in range(7)]

    # don't need the try/except, because I want the error to
    # be raised here so I can catch it outside the function
    data_type, id_type = types[rows[3][1]], types[rows[4][1]]

    # we want all four because you use the bases in your
    # if blocks later
    return (data_base, data_str), (id_base, id_str)

Now, that portion of the loop will look like this (after skipping some code):


    with open(filename) as csv_in, with open(new_filename, 'w') as csv_out:
        reader = csv.reader(csv_in)
        writer = csv.writer(csv_out)

        # use the function here
        try:
            # unpack the tuples
            (data_base, data_str), (id_base, id_str) = data_and_id_type(reader)
        except KeyError:
            print('Invalid type!')
            continue

        for row in reader:
            # rest of loop

The main benefits here are that I only keep values in global scope that I need, and I can drop the rest away. It encapsulates functionality in a way that makes the code easier to break into manageable pieces when reading it and/or maintaining it.

Also, by avoiding the list(reader) call, you don't incur a penalty of iterating over the whole file just to check the first 8 lines.

ID

Given that I don't know what all of your files look like, we can use the int function to verify that the byte values are indeed valid integers. The int function raises one of two exceptions: TypeError and ValueError. I'd catch both to be safe, and catch them explicitly for, well, explicitness. I'm also renaming ID to _id. Why not id? Because id is a built-in function name, and it's suggested in PEP-8 that the underscore should be used to avoid shadowing in favor of slight changes to a built-in name:

try:
    # also note the space between the comma and the second argument
    _id = f'{format(row[2], id_str)}'
    int(_id)
# the parentheses will capture both exceptions
except (ValueError, TypeError):
    _id = '0'

if, elif, elif, elif...

This is a bit unclear to me, because it looks like for each row, you keep the values that you might have captured earlier and write them into your csv_out. Small example:

a, b, c = 0, 0, 0

for i in range(3):
    if i == 0:
        a = 5
    elif i == 1:
        b = 6
    else:
        c = 777
    print(a, b, c)

5 0 0
5 6 0
5 6 777

The changes made for a and b are persisted through to the check for c. I'm not sure if that's your intent or not, so I'll leave that alone. The cleanup here is mostly style-related, again.

Parentheses around boolean checks

You don't need most of these. If a boolean check is large enough to warrant parentheses, it might need to be re-factored into either a variable or a function:

# go from this
if (_id == 0xcf11005):

# to this
if _id == '0xcf11005':

since the values for _id will be strings. Now is where that id_base and type_base will come into play:

def int_to_base(x, y):
    # this function removes a lot of redundancy in later if statements
    # and in new_var
    val = (int(x, data_base) << 8) | (int(y, data_base))
    return val / 10

def new_var(row, data_base):
    # this will produce values for var using
    # slicing over row
    for i, j in zip(row[6:13:2], row[5:12:2]):
        yield int_to_base(i, j)


for filename in flist:
~snip~
            if _id == '0xcf11005':
                # Refactor this into a function that returns a new var
                # since it's being overwritten anyways
                var = list(new_var(row, data_base))

            elif _id == '0xcf11505':
                var4 = int(row[5], data_base)

            elif _id == '0xcf11605':
                var8 = bin(int(row[5], data_base))
                var9 = bin(int(row[6], data_base))

            elif _id == '0xcf11a05':
                var10 = row[5]

            elif _id == '0xcf11e05':
                # looks nicer now, easy to read
                var11 = int_to_base(row[6], row[5])
                var12 = int_to_base(row[8], row[7])
                var13 = int_to_base(row[10], row[9])

            elif _id == '0xcf11f05':
                var14 = int(row[5], data_base)
                # note the added whitespace for the - operator
                var15 = int(row[6], data_base) - 40
                var16 = int(row[7], data_base) - 40

            else:
                continue

As a footnote, PEP-8 is something to be taken as (to paraphrase) "more of a guideline than an actual rule." Use the style guide where it makes sense, it can help clean up code and make it faster and/or more readable in a lot of places. However, blindly applying it can make your codebase worse. I've linked PEP-8 as a source in my answer, read it, consider it, but don't stress about it.

| improve this answer | |
\$\endgroup\$

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.