4
\$\begingroup\$

I have written the following code to export to data to a spreadsheet with this result: output

Please let me know how to improve it.

import numpy as np
import xlwt #THIS MAY BE GOOD
#from pandas import DataFrame

amin =2
amax=10
da = 2
names = ['amin', 'amax', 'da']
values = [amin, amax, da]

fname='EXPORTexample.xls'; sheetname = 'sheet 1'

book = xlwt.Workbook()
sh = book.add_sheet(sheetname)

col=0;
# FIRST WRITE THE VARIABLES AND THEIR VALUES INTO XLSX.
for row, (name, value) in enumerate(zip(names, values)):
    sh.write(row, col, name);
    sh.write(row, col+1, value); 

t2 = np.arange(0.0, 5.0, 1)
amat= np.arange(amin,amax,da)

fn=np.zeros((np.shape(t2)[0],1))

col=3;
#row_eps =0;
#row_Dt += row_eps;

for a in amat:
    fn = a*t2

    names = ['eps_dot', 'Delta_t']
    values=[a*1., a*1.]  
    #write specific a conditions on every data curve/set   
    for row, (name, value) in enumerate(zip(names, values)):
        sh.write(row, col, name);
        sh.write(row, col+1, value); 

    #write names such as strain, stress, d11s, d_dot, d11
    occ_rows = len(names)#no.of occupied rows. 
    names = ['strain', 'stress', 'd11s', 'd_dot', 'd11']
    for cols,name in enumerate(names):
        sh.write(occ_rows+1, col+cols, name)

    ## WRITE actual numbers
    for rows, (strain_i,stress_i, d11s_i, d_dot_i, d11_i) in enumerate(zip(fn, t2, fn, t2, fn)):

        sh.write(occ_rows+1+rows+1, col, strain_i)    
        sh.write(occ_rows+1+rows+1, col+1, stress_i)    
        sh.write(occ_rows+1+rows+1, col+2, d11s_i)    
        sh.write(occ_rows+1+rows+1, col+3, d_dot_i)    
        sh.write(occ_rows+1+rows+1, col+4, d11_i)    

    col+=6 #incremening column value to avoid overwriting and related error

book.save(fname)
\$\endgroup\$
1
\$\begingroup\$

First you should clean up your code - add whitespace, remove unnecessary comments, don't use semicolons, store repeated computations in variables, move repeated code fragments into functions, etc.

Second, if at all possible give them more descriptive names - I have no idea what this is about. A good way to do this is to figure out the different sections (you have them labelled with comments) and then turn each section into a well-named, small function. This way you can clearly document what each section does and why, without having to add comments that clutter everything up. Prefer expressing yourself in code over in comments whenever possible. Some people even consider writing comments a shameful act - I don't personally go that far, but here are some good articles about it:

Robert C Martin talks a lot about this - I'd highly recommend reading his work.

Comments are, at best, a necessary evil, nothing to celebrate The proper use of comments is to compensate for our failure to express ourself through the code itself http://www.cvc.uab.es/shared/teach/a21291/temes/coding_style/slides/comments.pdf

Third, use the right data structure for the job. Anytime you have name-value pairs and you aren't using a dictionary you're probably doing something wrong. Whenever you have a list but you never mutate it, you should make it a tuple.

Fourth, if you use the start keyword argument to enumerate you can get rid of a lot of extra code. You can then compress your data curves section pretty easily.

Once you've moved things into some nice clean functions, you can then put your runtime code into an if __name__ == '__main__' block.

Last, I'd make a wrapper that turns your workbook into a context manager. This way you can make sure that the workbook is saved, and it makes things quite a bit cleaner.

You might also want to consider adding support for command line arguments, making it easier to generate a lot of sheets like this that operate on different parameters.

I ended up with something like this

import numpy as np
import xlwt 
import contextlib


def write_to_sheet(sheet, row, column, name, value):
    sheet.write(row, column, name)
    sheet.write(row, column + 1, value)

def write_variables(sheet, variables):
    write_dict(sheet, 0, variables)

def write_conditions(sheet, column, conditions):
    write_dict(sheet, column, conditions)

def write_dict(sheet, column, dict_):
    for row, (name, value) in enumerate(dict_.items()):
        write_to_sheet(sheet, row, column, name, value)

def write_data_curve(sheet, data_curve, start_row, start_column):
    for column, (header, data) in enumerate(data_curve.items(), start=start_column):
        sheet.write(start_row, column, header)
        for row, value in enumerate(data, start=start_row + 1):
            sheet.write(row, column, value)

def write_data_curves(sheet, amin, amax, da):
    t2 = np.arange(0.0, 5.0, 1)
    amat = np.arange(amin, amax, da)
    col = 3

    for a, fn in ((a, a * t2) for a in amat):
        conditions = {
            'eps_dot': a * 1.,
            'Delta_t': a * 1.
        }
        write_conditions(sheet, col, conditions)

        data_curve = {'strain': fn, 'stress': t2,
                      'd11s': fn, 'd_dot': t2,
                      'd11': fn}
        write_data_curve(sheet, data_curve, len(conditions) + 1, col)

        col += 6

@contextlib.contextmanager
def open_book(filename):
    workbook = xlwt.Workbook()
    yield workbook
    workbook.save(filename)   


if __name__ == '__main__':
    amin = 2
    amax = 10
    da = 2

    variables = {
        'amin': amin,
        'amax': amax,
        'da': da
    }

    with open_book('EXPORTexample.xls') as book:
        sh = book.add_sheet('sheet 1')
        write_variables(sh, variables)
        write_data_curves(sh, amin, amax, da))
\$\endgroup\$
  • \$\begingroup\$ I don't think it's worth the trouble to create one-line functions like write_variables() when write_dict(sheet, 0, variables) convenes essentially the same information without the overhead—both at coding-time and run-time. \$\endgroup\$ – martineau Jul 15 '16 at 16:44
  • \$\begingroup\$ @martineau I can understand that the one-line functions don't do much. That being said, I think having the names is more important in the way of self-documentation (also it doesn't bind them to using a dict the way something named write_dict does). I think worrying about overhead for either coding or runtime for something like this is silly - there isn't enough of either to be a problem. \$\endgroup\$ – Dannnno Jul 15 '16 at 23:41
  • \$\begingroup\$ Then I guess we disagree. One-line functions that do nothing but add overhead are a waste of time in any language and since Python is interpreted, runtime is always a consideration. Now that I think about it more, the write_dict() function is not only poorly named, since it couples the name of the container data type with its purpose, it also isn't going to work as intended because of a fundamentally incorrect (and naive IMO) assumption it makes about that data structure. \$\endgroup\$ – martineau Jul 16 '16 at 1:46
  • \$\begingroup\$ Runtime is never a consideration in any language unless it has to be. Until something runs too slowly, it runs fast enough. Now you should obviously not write obtuse or unnecessarily slow code, but theres no reason to care about all of the little details before you need to. Theres a reason I only call write_dict inside of the more appropriately named functions - I suppose I could have called it _write_dict instead to emphasize that it is for internal use only. The assumption it makes about the data structure is fine - its something that has a method items which returns a key-value tuple. \$\endgroup\$ – Dannnno Jul 16 '16 at 2:08
  • \$\begingroup\$ If you change the data structures you use such that they have a different interface it seems obvious to me that you'll need to change your implementation. \$\endgroup\$ – Dannnno Jul 16 '16 at 2:09
0
\$\begingroup\$

@Dannnno and @martineau Thank you for your time and sorry for my late response. In my actual code, I have defined a subroutine, with all the necessary data as below:

def ExportDataSubroutine(fname, sh, col, eps_dot, delay_a, delay_tau, nsteps, strain, stress, d11s, d_dot, d11, Dt):

instead of so many subroutines. Also thanks for the links those will be certainly of use.

\$\endgroup\$
  • \$\begingroup\$ If you create an actual account with a linked email address and merge these others into it, you would be able to comment on your own post or update it. As stands, this seems light on review to be an answer. \$\endgroup\$ – mdfst13 Jul 22 '16 at 6:48

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.