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."
2 Answers 2
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:
# ...
-
\$\begingroup\$ Thank you. Your explanation is quite what I was looking for, to write code that is "more Pythonic". \$\endgroup\$dotancohen– dotancohen2012年07月22日 10:38:20 +00:00Commented Jul 22, 2012 at 10:38
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.
-
\$\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– dotancohen2012年07月22日 10:37:26 +00:00Commented Jul 22, 2012 at 10:37