Is there a better way to feed different parameters into a function with if-statements?

Question

I've been teaching myself Python for a little while now, and I've never programmed before. I just wrote a basic backup program that writes out the progress of each individual file while it is copying. I wrote a function that determines buffer size so that smaller files are copied with a smaller buffer, and bigger files are copied with a bigger buffer. The way I have the code set up now doesn't seem very efficient, as there is an if loop that then leads to another if loops, creating four options, and they all just call the same function with different parameters.

import os
import sys

def smartcopy(filestocopy, dest_path, show_progress = False):
    """Determines what buffer size to use with copy()
       Setting show_progress to True calls back display_progress()"""
    #filestocopy is a list of dictionaries for the files needed to be copied
    #dictionaries are used as the fullpath, st_mtime, and size are needed
    if len(filestocopy.keys()) == 0:
        return None
    #Determines average file size for which buffer to use
    average_size = 0
    for key in filestocopy.keys():
        average_size += int(filestocopy[key]['size'])
    average_size = average_size/len(filestocopy.keys())
    #Smaller buffer for smaller files
    if average_size < 1024*10000: #Buffer sizes determined by informal tests on my laptop
        if show_progress:
            for key in filestocopy.keys():
                #dest_path+key is the destination path, as the key is the relative path
                #and the dest_path is the top level folder
                copy(filestocopy[key]['fullpath'], dest_path+key, 
                     callback = lambda pos, total: display_progress(pos, total, key))
        else:
            for key in filestocopy.keys():
                copy(filestocopy[key]['fullpath'], dest_path+key, callback = None)
   #Bigger buffer for bigger files 
   else:
        if show_progress:
            for key in filestocopy.keys():
                copy(filestocopy[key]['fullpath'], dest_path+key, 1024*2600, 
                     callback =  lambda pos, total: display_progress(pos, total, key))
        else:
            for key in filestocopy.keys():
                copy(filestocopy[key]['fullpath'], dest_path+key, 1024*2600)

def display_progress(pos, total, filename):
    percent = round(float(pos)/float(total)*100,2)
    if percent <= 100: 
        sys.stdout.write(filename + ' - ' + str(percent)+'%   \r')
    else:
        percent = 100
        sys.stdout.write(filename + ' - Completed \n')

Is there a better way to accomplish what I'm doing? Sorry if the code is commented poorly or hard to follow. I didn't want to ask someone to read through all 120 lines of my poorly written code, so I just isolated the two functions. Thanks for any help.

Answer 1

WoLpH and Sapph's answers are about right. Here's a random Python simplification you can do for your average calculation using generator expressions:

average_size = sum(int(filestocopy[k]['size']) for k in filestocopy)/len(filestocopy) 

Answer 2

You gave a huge example code and it was difficult to follow. But what you are asking (interpreting) from your question's Title is "How does one normally pass variable number of arguments" to a python's function call? Then the answer is by passing lists/tuples or dictionaries.

def fun(a, *b, **c):
    print a
    for item in b:
        print item
    for k,v in c:
        print k,v

a = 42
b = [1,2,3]
c = {'a':'1','b':2,'c':True}

fun(a,b,c)

Just note that when passing container (list/tuple) and a dictionary, the former should precede the later.

Answer 3

I believe that you are on the right track. One solution to your problem is saving the parameters in variables.

def smartcopy(filestocopy, dest_path, show_progress = False):
    """Determines what buffer size to use with copy()
       Setting show_progress to True calls back display_progress()"""
    #filestocopy is a list of dictionaries for the files needed to be copied
    #dictionaries are used as the fullpath, st_mtime, and size are needed
    if len(filestocopy.keys()) == 0:
        return None
    #Determines average file size for which buffer to use
    average_size = 0
    for key in filestocopy.keys():
        average_size += int(filestocopy[key]['size'])
    average_size = average_size/len(filestocopy.keys())
    #Smaller buffer for smaller files

    if show_progress:
        progress_callback = lambda pos, total: display_progress(pos, total, key)
    else:
        progress_callback = None

    #Bigger buffer for bigger files 
    if average_size < 1024*10000: #Buffer sizes determined by informal tests on my laptop
        buffer = None
    else:
        buffer = 1024 * 2600

    for key, value in filestocopy.iteritems():
        #dest_path+key is the destination path, as the key is the relative path
        #and the dest_path is the top level folder
        copy(value['fullpath'], dest_path+key, buffer, callback=progress_callback)

Or another solution if you want to keep the normal default arguments:

def smartcopy(filestocopy, dest_path, show_progress = False):
    """Determines what buffer size to use with copy()
       Setting show_progress to True calls back display_progress()"""
    #filestocopy is a list of dictionaries for the files needed to be copied
    #dictionaries are used as the fullpath, st_mtime, and size are needed
    if len(filestocopy.keys()) == 0:
        return None
    #Determines average file size for which buffer to use
    average_size = 0
    for key in filestocopy.keys():
        average_size += int(filestocopy[key]['size'])
    average_size = average_size/len(filestocopy.keys())
    #Smaller buffer for smaller files

    kwargs = {}
    if show_progress:
        kwargs['callback'] = lambda pos, total: display_progress(pos, total, key)

    #Bigger buffer for bigger files 
    if average_size >= 1024*10000: #Buffer sizes determined by informal tests on my laptop
        kwargs['buffer'] = 1024 * 2600

    for key, value in filestocopy.iteritems():
        #dest_path+key is the destination path, as the key is the relative path
        #and the dest_path is the top level folder
        copy(value['fullpath'], dest_path+key, **kwargs)

A little extra note, something like this:

if len(filestocopy.keys()) == 0:
    return None

Could also be written like this:

if not filestocopy:
    return

The loop itself could be simplified to this:

for key, value in filestocopy.iteritems():
        #dest_path+key is the destination path, as the key is the relative path
        #and the dest_path is the top level folder
        copy(value['fullpath'], dest_path+key, **kwargs)

And the keys() is never really needed when iterating dictionaries as that's the default behaviour :)

So, the following lines will all get the same results:

keys = list(some_dict)
keys = some_dict.keys()
keys = list(some_dict.keys())
keys = list(some_dict.iterkeys())

Answer 4

Well it looks like you're calling the same function copy

    if show_progress:
        for key in filestocopy.keys():
            copy(filestocopy[key]['fullpath'], dest_path+key, 1024*2600, 
                 callback =  lambda pos, total: display_progress(pos, total, key))
    else:
        for key in filestocopy.keys():
            copy(filestocopy[key]['fullpath'], dest_path+key, 1024*2600)

with different number of arguments. Why do you need the if/else here. It's not very clear.

Answer 5

Some notes, first:

• Instead of len(filestocopy.keys()), you can just do len(filestocopy)
• Prefer for key in filestocopy.iterkeys() over for key in filestocopy.keys()

You could try something like:

callback = None
if show_progress:
    callback = lambda pos, total: display_progress(pos, total, key)

if average_size < 1024*10000:
    for key in filestocopy.keys():
        copy(filestocopy[key]['fullpath'], dest_path + key, callback)
else:
    for key in filestocopy.keys():
        copy(filestocopy[key]['fullpath'], desth_path + key, 1024 * 2600, callback)

I don't know what arguments your 'copy' function takes, but you could do something instead:

callback = None
buffer_sz = 1024 * 2600 if average_size >= 1024*10000 else WHATEVER_SIZE

if show_progress:
    callback = lambda pos, total: display_progress(pos, total, key)

for key in filestocopy.keys():
    copy(filestocopy[key]['fullpath'], dest_path + key, buffer_sz, callback)

Here, "WHATEVER_SIZE" should obviously be replaced with the buffer size for smaller lists, or whatever the default value is supposed to be.

The basic idea is to initialize your function arguments before you loop, to variables, and then use those variables in your function call. :)