2
\$\begingroup\$

Here is a simple Python script that I wrote with lots of help from the Python mailing list. I moves all the files of subdirectories to the top level directory. It works well for my use, but if there are things that can be improved then I would like to know. I ask in the interest of being a better Python programmer and learning the language better. Thanks.

#!/usr/bin/python
# -*- coding: utf-8 -*-
import sys
import os
import shutil

currentDir = os.getcwd()
forReal = False

if ( currentDir in ['/home/dotancohen', '/home/dotancohen/.bin'] ):
        print "Error: Will not run in "+currentDir
        exit()

if ( len(sys.argv)>1 and sys.argv[1] in ['m', 'M', 'c', 'C'] ):
        forReal = True
else:
        print "\nThis is a test run. To actually perform changes, add 'm' (move) or 'c' (copy) after the program name.\n"

filesList = os.walk(currentDir)

for rootDir, folders, files in filesList:
        for f in files:
                if (rootDir!=currentDir):
                        toMove = os.path.join(rootDir, f)
                        print toMove
                        newFilename = os.path.join(currentDir,f)
                        renameNumber = 1
                        while(os.path.exists(newFilename)):
                                newFilename = os.path.join(currentDir,f)+"_"+str(renameNumber)
                                renameNumber = renameNumber+1
                        if ( forReal and sys.argv[1] in ['m', 'M'] ):
                                os.rename(toMove, newFilename)
                        elif ( forReal and sys.argv[1] in ['c', 'C'] ):
                                shutil.copy(toMove, newFilename)

if ( not forReal ):
        print "\nThis was a test run. To actually perform changes, add 'm' (move) or 'c' (copy) after the program name."
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Peter has already mentioned putting the code into methods. This isn’t just a personal preference, it’s a general recommendation that all Python code should follow.

In fact, consider the following an idiomatic Python code outline:

import sys

def main():
    # …

if __name__ == '__main__':
    sys.exit(main())

Implement PEP 8 – Style Guide for Python Code. In particular, use underscore_separated variables instead of camelCase.

Programs should follow the general flow (1) input, (2) processing, (3) output. Argument reading is part of (1); consequently, do it only once, at the start.

Set variables depending on that input.

perform_move = sys.argv[1] in ['M', 'm']
perform_copy = sys.argv[1] in ['C', 'c']
dry_run = not perform_copy and not perform_move
…

Note that I’ve inverted the conditional for forReal. First of all, because it’s more common (“dry run” is what this is usually called). Secondly, the name forReal is rather cryptic.

Put the ['/home/dotancohen', '/home/dotancohen/.bin'] into a configurable constant at the beginning of the code, or even into an external configuration file. Then make it a parameter to the method which performs the actual logic.

By contrast, the filesList variable is unnecessary. Just iterate directly over the result of os.walk.

Operations of the form renameNumber = renameNumber+1 should generally be written as rename_number += 1. Also, pay attention to consistent whitespace usage.

I’d put the logic to find a unique target path name into its own method unique_pathname. Then I’d replace the concatenation by string formatting.

def unique_pathname(basename):
    unique = basename
    rename_no = 1
    while os.path.exists(unique):
        unique = '{0}_{1}'.format(basename, rename_no)
    rename_no += 1

    return unique

Notice that I’ve also removed the redundant parenthese around the loop condition here. The same should be done for all your if statements.

if ( forReal and sys.argv[1] in ['m', 'M'] ):

The test for forReal here is redundant anyway, it’s implied in the second condition. But we’ve got variables now anyway:

if perform_move:
    # …
elif perform_copy:
    # …
\$\endgroup\$
1
  • \$\begingroup\$ Thank you. Your explanation is quite what I was looking for, to write code that is "more Pythonic". \$\endgroup\$
    – dotancohen
    Jul 22, 2012 at 10:38
2
\$\begingroup\$

I'll typically wrap the main body of my code in a python script in a function and break the code apart into more functions, allowing me to reuse that code later.

import os
import shutil
import sys

def flattenDirectory(location, action, actuallyMoveOrCopy=True):
   """ This function will move or copy all files from subdirectories in the directory location specified by location into the main directory location folder
       location - directory to flatten
       action - string containing m(ove) or M(ove) or c(opy) or C(opy)"""
   #Your code here

if __name__ == "__Main__":
    #same code to find currentDirectory here
    actuallyMoveOrCopy = len(sys.argv)>1 and sys.argv[1] in ['m', 'M', 'c', 'C']
    flattenDirectory(currentDirectory, sys.argv[1], actuallyMoveOrCopy)

Now you can still run this code as you normally would, but should you need it in another script you can use it in an import without having to worry about the code automatically running at the time of import.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you. This is a good example of a script that likely will be called from another program someday, so it is a good fit to this approach. \$\endgroup\$
    – dotancohen
    Jul 22, 2012 at 10:37

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.