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
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
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.
