CODEWITHSUNDEEP
python
blu
blu

Reputation: 13175

Is there a more Pythonic approach to this?

This is my first python script, be ye warned.

I pieced this together from Dive Into Python, and it works great. However since it is my first Python script I would appreciate any tips on how it can be made better or approaches that may better embrace the Python way of programming.

import os
import shutil

def getSourceDirectory():
    """Get the starting source path of folders/files to backup"""
    return "/Users/robert/Music/iTunes/iTunes Media/"

def getDestinationDirectory():
    """Get the starting destination path for backup"""
    return "/Users/robert/Desktop/Backup/"

def walkDirectory(source, destination):
    """Walk the path and iterate directories and files"""

    sourceList = [os.path.normcase(f)
        for f in os.listdir(source)]

    destinationList = [os.path.normcase(f)
        for f in os.listdir(destination)]

    for f in sourceList:
        sourceItem = os.path.join(source, f)
        destinationItem = os.path.join(destination, f)  

        if os.path.isfile(sourceItem):
            """ignore system files"""
            if f.startswith("."):
                continue

            if not f in destinationList:
                "Copying file: " + f
                shutil.copyfile(sourceItem, destinationItem)

        elif os.path.isdir(sourceItem):
            if not f in destinationList:
                print "Creating dir: " + f
                os.makedirs(destinationItem)

            walkDirectory(sourceItem, destinationItem)

"""Make sure starting destination path exists"""
source = getSourceDirectory()
destination = getDestinationDirectory()

if not os.path.exists(destination):
    os.makedirs(destination)

walkDirectory(source, destination)

Upvotes: 3

Views: 311

Answers (5)

Tim McNamara
Tim McNamara

Reputation: 18375

The main thing to make things more Pythonic is to adopt Python's PEP8, the style guide. It uses underscore for functions.1

If you're returning a fixed string, e.g. your get* functions, a variable is probably a better approach. By this, I mean replace your getSourceDirectory with something like this:

source_directory = "/Users/robert/Music/iTunes/iTunes Media/"

Adding the following conditional will mean that code that is specific for running the module as a program does not get called when the module is imported.

if __name__ == '__main__':
    source = getSourceDirectory()
    destination = getDestinationDirectory()

    if not os.path.exists(destination):
        os.makedirs(destination)

    walkDirectory(source, destination)

I would use a try & except block, rather than a conditional to test if walkDirectory can operate successfully. Weird things can happen with multiple processes & filesystems:

try:
    walkDirectory(source, destination)
except IOError:
    os.makedirs(destination)
    walkDirectory(source, destination)

1 I've left out discussion about whether to use the standard library. At this stage of your Python journey, I think you're just after a feel how the language should be used in general terms. I don't think knowing the details of os.walk is really that important right now.

Upvotes: 1

msw
msw

Reputation: 43487

The code

  • has no docstring describing what it does
  • re-invents the "battery" of shutil.copytree
  • has a function called walkDirectory which doesn't do what its name implies
  • contains get* functions that provide no utility
    • those get functions embed high-level arguments deeper than they ought
  • is obligatorily chatty (print whether you want it or not)

Upvotes: 4

paprika
paprika

Reputation: 2484

As others mentioned, you probably want to use walk from the built-in os module. Also, consider using PEP 8 compatible style (no camel-case but this_stye_of_function_naming()). Wrapping directly executable code (i.e. no library/module) into a if __name__ == '__main__': ... block is also a good practice.

Upvotes: 6

JoshD
JoshD

Reputation: 12814

I recommend using os.walk. It does what it looks like you're doing. It offers a nice interface that's easy to utilize to do whatever you need.

Upvotes: 2

Charlie Martin
Charlie Martin

Reputation: 112366

Use os.path.walk. It does most all the bookkeeping for you; you then just feed a visitor function to it to do what you need.

Or, oh damn, looks like os.path.walk has been deprecated. Use os.walk then, and you get

for r, d, f in os.walk('/root/path')
    for file in f:
       # do something good.

Upvotes: 3

Related Questions