current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite

My question concerns the following Python code which is already working. As far as I have seen, there are very elegant solutions of compacting code. Do you have any ideas on how to make the following code look smoother? 

mom = [0.,0.13,0.27,0.53,0.67]
strings = ['overview_files/root/file_' + str(e) for e in mom] 
myfile = [np.loadtxt(s) for s in strings]
nbinp = len(mom)
nbinomega = len(myfile[0][:,0])

x, y, z = (np.empty(nbinp*nbinomega) for i in range(3))
for i in range(nbinomega):
  for j in range(nbinp):
    i_new = i + j*nbinomega
    y[i_new] = myfile[j][i,0] - 1.4
    x[i_new] = mom[j]
    z[i_new] = myfile[j][i,1]
share|improve this question
1  
Can you describe what you think is not smooth about this code? It is only 12 lines and is already fairly terse with liberal use of list comprehension. – unholysampler Feb 4 '14 at 14:56
2  
Can you explain what this code is supposed to do? – Gareth Rees Feb 4 '14 at 14:56
    
@unholysampler: Well, I feel that this c++-type for loops are not really the python way. – user3058865 Feb 4 '14 at 15:30
    
@GarethRees: It is supposed to prepare data for a contour plot with matplotlib. Data is saved as textfiles with format omega-f(omega,p) in different files for different ps. – user3058865 Feb 4 '14 at 15:31

1 Answer 1

up vote 3 down vote accepted

You may want to follow PEP8 a bit more closely to learn how to write code that is easily understood by most Python developers. Eg. mom = [0.3, 0.13] instead of mom = [0.3,0.13] and four spaces indentation.

Try to be more careful about variable names. I couldn't understand what most of them meant, which is probably because I don't much about the code you're writing. But think about your readers (including you in three months) and wonder what are the best ways to convey information in your variable names. For example, 'myfile' suggest that this is not a collection. And 'my' doesn't provide any useful information.

There's a common idiom in Python to avoid dealing with ranges explicitely: enumerate().

for i, this in enumerate(myfile):
    for j, that in enumerate(myfile[i]):
        i_new = ...
        y[i_new] = ...
        x[i_new] = ...
        z[i_new] = ...

If you often deal with ranges, it will certainly help you at some point.

share|improve this answer

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.