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.

I have around ~2M of files in one folder, which I need to manage. So, I want to separate them in groups by their filetypes.

Example:

filenames

1.A, 2.A, 3.A, 2.B, 3.B, 4.B, 5.C, 6.C

My code:

import os
all_files = os.listdir(".")
filetypes = list(set([i.split(".")[-1] for i in all_files]))
for i in filetypes:
    os.system("mkdir -p %s"%i)
    os.system("mv *.%s %s"%(i, i))

How can I improve this?

share|improve this question

2 Answers 2

up vote 2 down vote accepted

You could make this a bit more robust. You might have control over what is generated, but it won't hurt to make sure of a few things:

Filter out any non-files:

 all_files = (x for x in os.listdir() if os.path.isfile(x))

You can iterate over a set just fine, no need to convert it back to a list. Further, os already has a splitext method that'll separate a filename into (name, ext), and will give the ext as '' if it doesn't exist (currently, your code will crash if there is no extension):

filetypes = set((os.path.splitext(f)[1] for f in all_files]))

There is also a move command in shutil that you could use instead of a call to os.system(), which is likely to be a lot faster (and safer). To generate the file names, you can use glob:

for ftype in filetypes[1:]:
    os.mkdir(ftype)
    for fname in glob.iglob('*.{0}'.format(ftype)):
        shutil.move(fname, ftype) 

If these files are somewhat important, you might want to consider copying and then delete them instead of moving them directly.

share|improve this answer

Avoid os.system() by all means. It wastes resources by creating 2 processes per invocation, and is vulnerable to various attacks (at least spell out full paths to the mkdir and mv utilities).

os.mkdir() provide all necessary functionality (don't forget to catch OSError). A combination of os.link() and os.unlink() emulates mv (they are only available on Unix - but you are calling mv anyway).

share|improve this answer
1  
also worse: using os.system("mv *.%s %s"%(i,i)) when there are filenames with spaces in them can cause catastrophic consequences since part of the filename can be interpreted as a shell command. –  Emanuele Paolini Aug 18 at 7:45

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.