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."
asked Jul 20, 2012 at 14:16
\$\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:
 # ...
answered Jul 20, 2012 at 17:04
\$\endgroup\$
1
  • \$\begingroup\$ Thank you. Your explanation is quite what I was looking for, to write code that is "more Pythonic". \$\endgroup\$ Commented 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.

answered Jul 20, 2012 at 16:14
\$\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\$ Commented Jul 22, 2012 at 10:37

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.