current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite
1

I was tasked with creating a script to be able to rename some files and then move them into different folders based on a code in the filename.

import os
import shutil

path = (r"C:\user\reports")

filelist = [ f for f in os.listdir(path) if f.endswith(".xml") ]

for f in filelist:
    x = os.path.join(path, f)
    os.remove(x)

filelist = [ f for f in os.listdir(path) if f.endswith(".pdf") ]


for f in filelist:
    os.rename(os.path.join(path, f),os.path.join(path, f[31:]))


filelist = [f for f in os.listdir(path) if f.endswith(".pdf")]

for f in filelist:    
    if "A" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderA")
    if "B" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderB")
    if "C" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderC")
    if "D" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderD")
    if "E" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderE")
    if "F" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderF")
    if "G" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderG")
    if "H" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderH")
    if "I" in f[-6:]: shutil.copy(os.path.join(path, f),r"C:\user\reports\folderI") 

filelist = [f for f in os.listdir(path) if f.endswith(".pdf")]

for f in filelist:
    x = os.path.join(path, f)
    os.remove(x)

First, it removes any XML files in the folder. Then it strips the first 31 characters (it will always be 31 characters) of the filename, leaving just a name and a code. It then checks if the final 6 characters contain a "code" and then move the file to the correct folder based off that code. After every file has been moved, it will delete every file in the parent folder.

I know my current code could be improved, but it's been a couple of years since I've done Python so I've literally done it the easiest way (from what I could Google) possible. Any improvements or suggestions to what features of Python I should be looking at would be great.

share|improve this question
add comment

2 Answers

up vote 6 down vote accepted

Don't reinvent the wheel

For getting the files with a specific extension you should use glob:

xmlFiles = glob.glob('*.xml')

(Note that this will work relative to the current path.)

Avoid code duplication

You have some serious code duplication going on, replace it my appropriate code like:

#I don't really know the semantics of your paths so you should rename this accordingly
allowedPaths = 'ABCDEFGHI'
for filename in filelist:    
    for character in allowedPaths:
        if character in f[-6:]: shutil.copy(os.path.join(path, f), 
                                            os.join.path(path, r"folder" + character))

Magic numbers

Avoid these magic numbers by replacing them with "constants"

PREFIX_LENGTH = 31
POSTFIX_LENGTH = 6
#...
os.rename(os.path.join(path, f),os.path.join(path, f[PREFIX_LENGTH:]))
#...
if character in f[-POSTFIX_LENGTH:]:
share|improve this answer
add comment
up vote 2 down vote

Watch out for bugs due to case. Since you reference a path C:\user\reports\folderX, I'm assuming you're running on Windows with a case-insensitive filesystem. However the code as you wrote it is very much case-sensitive. The two places this comes up most clearly are in your extension filtering, and in your PDF file name filtering. Depending on your exact needs, there are a couple approaches you could take:

  • Find files by using glob. As Nobody suggested, by using glob.glob, you will be running the comparison check in the same context as the filesystem.
  • Normalize the case. By checking, e.g. f.upper().endswith(".PDF") you ensure you are checking against the same types of letters that you have.
  • Use a case-insensitive tool. By using, for example, regular expressions with the ignore-case flag, you can ensure that the case of the filename doesn't matter.

This actually brings us to my next point. If you look closely at the check if "F" in f[-6:], you'll note that once you make this case insensitive, it will always be True. Why? There's an f in pdf. But you won't get there, because there's a d in pdf. How can you fix this?

  • Leverage os.path.splitext. Use os.path.splitext() to get your file's name and extension components. You can look specifically at the extension when you're filtering filetype, and specifically at the name when filtering for A-I tokens.
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.