6
\$\begingroup\$

I've written the following script

import shutil,os,glob

# setting up destination folders

PYDEST = "/home/ubuntu/python-scripts/python-test"

TEXTDEST = "/home/ubuntu/python-scripts/texte-test"

def move8files(pwd):
    PWD = pwd + "*"
    for files in glob.glob(PWD):
        if files.endswith(".txt"):
            print(files)
            shutil.move(files,TEXTDEST)
        elif files.endswith(".py"):
            shutil.move(files,PYDEST)

move8files("/home/ubuntu/python-scripts/")

Questions/Issues are the following:

  • How can I make it simpler, especially by using the os.listdir but with having the pathname (listdir does not give pathname)?
  • Would it be possible to include a part in my script, where instead of saying which folders are for python or text files, it will look syntaxically at specific folders containing words like python or texte in it?
  • What can I improve in my script, for better performance?
\$\endgroup\$
4
\$\begingroup\$

I have no idea, how to make it simpler. I could only offer an alternative:

#!/usr/bin/env python

import os
import shutil

fullpath = os.path.join
python_directory = "./py"
start_directory = "./mixed"
text_files = "./txt"


def main():
    for dirname, dirnames, filenames in os.walk(start_directory):
        for filename in filenames:
            source = fullpath(dirname, filename)
            if filename.endswith("py"):
               shutil.move(source, fullpath(python_directory, filename))
            elif filename.endswith("txt"):
                shutil.move(source, fullpath(text_files, filename))

if __name__ == "__main__":
    main()

I think os.walk()is what you are looking for.

\$\endgroup\$
  • 1
    \$\begingroup\$ Hi @thomas-junk, thanks for your feedback. Why did you use def main()? And what is if __name__ == "__main__":? \$\endgroup\$ – Andy K Aug 17 '16 at 6:53
  • \$\begingroup\$ Python files could be run in two modes: 1) they could be run on their own; then name is set to "main" or 2) they could (accidentally) be imported. And to prevent the interpreter to blindly run any code nonintentionally, you guard against that with the guarding clause if name == " main ", read: if you are running in standalone mode, run main(). \$\endgroup\$ – Thomas Junk Aug 17 '16 at 7:41
  • \$\begingroup\$ I messed up "dunder" in my comment above (thanks markdown :D). But you see, what I meant. \$\endgroup\$ – Thomas Junk Aug 17 '16 at 7:43
  • 1
    \$\begingroup\$ No worries @thomas-junk, I got your point ;) \$\endgroup\$ – Andy K Aug 17 '16 at 7:44
  • 1
    \$\begingroup\$ that os walk stuff is just great!!! \$\endgroup\$ – Andy K Aug 17 '16 at 7:50
2
\$\begingroup\$

Expanding @ThomasJunk's answer I think you can improve a bit by using a small dictionary, so you can add more file types if you need to:

#!/usr/bin/env python

import os
import shutil

fullpath = os.path.join
python_directory = "./py"
start_directory = "./mixed"
text_files = "./txt"

default_path = "./default"

dir_lookup = {
    'py': './py/',
    'txt': './txt/'
}

def get_extension(filename):
    return filename[filename.rfind('.')+1:]

def main():
    for dirname, dirnames, filenames in os.walk(start_directory):
        for filename in filenames:
            source = fullpath(dirname, filename)
            extension = get_extension(filename)
            if (extension in dir_lookup):
                shutil.move(source, fullpath(dir_lookup[extension], filename))
            else:
                shutil.move(source, fullpath(default_path, filename))

if __name__ == "__main__":
    main()

Also, a side note: you're working with relative paths, so be careful, you should add a few checks.

For example I would add a check to see that the destination directory actually exists.

I would also add a check that the directories are what they seem. If one or more of the destination directories is actually a link to another directory or even worse to a file, you may end up overwriting stuff.

EDIT: As suggested, I added a default case when there is no match in the dictionary. Checking that the key exists is one way, and it's probably good enough for such a simple program. Another way would be to use defaultdict, but I didn't want to make it too complicated.

\$\endgroup\$
  • \$\begingroup\$ and what happens with .pdf? \$\endgroup\$ – Thomas Junk Aug 17 '16 at 8:43
  • \$\begingroup\$ @ThomasJunk You're right, I can add a default case \$\endgroup\$ – ChatterOne Aug 17 '16 at 9:02
  • \$\begingroup\$ Hi @ChatterOne, what's the purpose of get_extension()? \$\endgroup\$ – Andy K Aug 17 '16 at 9:02
  • \$\begingroup\$ @AndyK get_extension() gets the extension of the filename. You could also not have the function and inline it, I just though it was easier to read. \$\endgroup\$ – ChatterOne Aug 17 '16 at 9:15
  • \$\begingroup\$ @ChatterOne I got the logic. Thanks. Bit tricky but it is interesting \$\endgroup\$ – Andy K Aug 17 '16 at 9:17

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.