Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I was wondering if there is a smarter way of doing the following code. Basically what is does is that it opens a data file with a lot of rows and columns. The columns are then sorted so each column is a vector with all the data inside.

"3.2.2 - Declare variables"
lineData    = list()

for line in File:
  splittedLine = line.split() # split
  lineData.append(splittedLine) #collect 

And here the fun begins

"3.2.3 - define desired variables from file"
col1    = "ElemNo"
col2    = "Node1"
col3    = "Node2"
col4    = "Length"
col5    = "Area"
col6    = "Inertia"
col7    = "Fnode1"
col8    = "Fnode2"
col9    = "SigmaMin"
col10   = "SigmaMax"

"3.2.3 - make each variable as a list/vector"
var ={col1:[], col2:[], col3:[], col4:[], col5:[], col6:[], col7:[], col8:[]
     ,col9:[],col10:[]}  

"3.2.3 - take the values from each row in lineData and collect them into the correct variable"
for row in lineData:
  var[col1] .append(float(row[0])      )    #[-]    ElemNo
  var[col2] .append(float(row[1])      )    #[-]    Node1
  var[col3] .append(float(row[2])      )    #[-]    Node2
  var[col4] .append(float(row[3])      )    #[mm]   Length
  var[col5] .append(float(row[4])      )    #[mm^2] Area
  var[col6] .append(float(row[5])*10**6)    #[mm^4] Inertia 
  var[col7] .append(float(row[6])      )    #[N]    Fnode1
  var[col8] .append(float(row[7])      )    #[N]    Fnode2
  var[col9] .append(float(row[8])      )    #[MPa]  SigmaMin
  var[col10].append(float(row[9])      )    #[MPa]  SigmaMax

As you see this is a rather annoying way of making each row into a variable. Any suggestions?

share|improve this question
add comment

3 Answers 3

up vote 8 down vote accepted

First of all don't create variables for those keys, store them in a list.

keys = ["ElemNo", "Node1", "Node2", "Length", "Area", "Inertia",
        "Fnode1", "Fnode2", "SigmaMin", "SigmaMax"]

You can use collections.defaultdict here, so no need to initialize the dictionary with those keys and empty list.

from collections import defaultdict
var = defaultdict(list)

Now, instead of storing the data in a list, you can populate the dictionary during iteration over File itself.

for line in File:
    for i, (k, v) in enumerate(zip(keys, line.split())):
        if i == 5:
            var[k].append(float(v)*10**6)
        else:
            var[k].append(float(v)) 
share|improve this answer
    
Maybe i == 5 -> k == 'Inertia'? –  tokland Feb 25 at 19:21
    
@tokland But you've already included that in your answer. If you don't mind then I can suggest that in my answer as well. –  undefined is not a function Feb 25 at 19:25
add comment

In functional approach, without refering to meaningless numeric indexes but column names, and creating a dictionary with "ElemNo" as keys instead of a dict of lists, I'd write:

columns = [
  "ElemNo", "Node1", "Node2", "Length", "Area", "Inertia",
  "Fnode1", "Fnode2", "SigmaMin", "SigmaMax",
]

def process_line(line):
    def get_value(column, value):
        if column == "Inertia":
            return float(value) * (10**6)
        else:
            return float(value)
    elem = {col: get_value(col, value) for (col, value) in zip(columns, line.split())}
    return (elem["ElemNo"], elem)

data = dict(process_line(line) for line in fd)
share|improve this answer
    
How would this be better from the first suggestion? –  Mikkel Grauballe Feb 25 at 13:34
2  
@MikkelGrauballe: 1) don't use numeric indexes, 2) functional approach, 3) use elemno as key so finding elements is easier. Maybe not better, but it's a different approach. –  tokland Feb 25 at 13:41
add comment

You can use zip to perform a transpose operation on a list of lists:

column_names = [
    "ElemNo", "Node1", "Node2", "Length", 
    "Area", "Inertia", "Fnode1", "Fnode2", 
    "SigmaMin", "SigmaMax"
]

columns = dict(zip(column_names, (map(float, col) for col in zip(*lineData))))
#                                        transposes lineData ^^^

columns["Inertia"] = [x * 10 ** 6 for x in columns["Inertia"]]
share|improve this answer
2  
I'm +1ing mainly for transforming Inertia separately from the transpose. It's a separate "rule", so I don't particularly like putting it inside the main transpose loop, however that loop was done. –  Izkata Feb 25 at 19:13
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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