3
\$\begingroup\$

File manipulation code, I've noticed, has two salient properties:

  1. It's everywhere, and vitally important to functional software
  2. It has lots of exceptions.

Please give me some pointers on how I can better improve the usability of this rough draft I wrote yesterday. It assumes some very basic things, and was scrapped together after browsing Google as I stumbled through the concepts.

makeproject.py

'''create a simple setup folder for a new project

Follows the guide posted at:
http://guide.python-distribute.org/quickstart.html#lay-out-your-project

The structure built will match the following:

| ProjectName/
|---> LICENSE.txt
    | README.txt
    | setup.py
    | projectname/
    | ---> __init__.py

#For the sake of best practices, __init__.py is left empty.
'''
#! usr/bin/env python

import sys, os, errno

class NewProject(object):
    '''interface for building a directory with a new project in it'''

    def __init__(self, projectname, directory):
        self.project = projectname
        self.base = check_path(directory)
        #project defaults: I don't want them here as class attributes!
        self.SETUP = {

'README.txt' :
'''This is an unmodified README text file.
It contains a list of changes to the program.
It also contains help for dealing with the application.
''',

'LISCENSE.txt' :
'''Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License
'''
}

    def newproject(self):
        '''creates a file structure for a new project at `directory`
        '''
        self.name = self.project.replace(' ', '')
        self.path = os.sep.join([self.base, self.name])

        sub = self.name.lower()
        subpath = os.sep.join([self.path, sub])

        check_build_path(subpath)
        for filename, content in self.SETUP.items():
            self.buildfile(filename, content, directory=self.path)
        #setup takes arguments, it has its own method
        setup = self.buildsetup()
        self.buildfile('setup.py', setup)
        self.buildfile('__init__.py', directory=subpath)        

    def buildfile(self, name, content="", directory = ""):
        '''opens and creates a new file at `directory` with `contents`'''
        #assumes bad directories have been purified
        if directory == "":
            loc = os.sep.join([self.path, name])
            w = open(loc, 'w')
        else:
            directory = os.sep.join([directory, name])
            w = open(directory, 'w')
        w.write(content)
        w.close()

    def buildsetup(self):
        return '''from distutils.core import setup

setup(
    name='{0}',
    version='0.1dev',
    packages=['{1}',],
    license=open('LISCENSE.txt').read(),
    long_description=open('README.txt').read(),
)
'''.format(self.project, self.name.lower())

def check_path(loc):
    '''recursively check if last character in `loc` is like os.sep character.
    If so, remove.'''
    if loc[-1] == os.sep:
        return check_path(loc[:-1])
    return loc

def check_build_path(loc):
    d = os.path.normpath(loc)
    if not os.path.exists(d):
        os.makedirs(d)
    else:
        try:
            os.rmdir(d)
        except OSError as ex:
            if ex.errno == errno.ENOTEMPTY:
                print "Directory specified must be new or empty"
                sys.exit(1)
        #if delete was successful, build the directory
        check_build_path(loc)

if __name__ == '__main__':
    if len(sys.argv) != 3:
        print "Usage: string 'Project Name', string '/abs/path'"
        sys.exit(1)
    project = NewProject(sys.argv[1], sys.argv[2])
    project.newproject()
\$\endgroup\$
4
  • \$\begingroup\$ What do you mean by "operating system code"? \$\endgroup\$ Dec 25 '11 at 2:56
  • \$\begingroup\$ Code that manages operating system resources: directories, files, permissions, etc. To me, it is one of the lines that separates toy scripts from actual work. \$\endgroup\$
    – Droogans
    Dec 26 '11 at 15:01
  • \$\begingroup\$ Unusual definition, one that I don't understand. To me OS code is part of the OS. I don't know what a "toy script" is--if it does useful work, it's not a toy. \$\endgroup\$ Dec 26 '11 at 15:03
  • \$\begingroup\$ Toy scripts are for proof of concept or trying out a new branch of study. It doesn't mean it's useless, just not production quality. \$\endgroup\$
    – Droogans
    Dec 26 '11 at 15:17
3
\$\begingroup\$
'''create a simple setup folder for a new project

The recommendation is to always use """ not ''' for docstrings

Follows the guide posted at:
http://guide.python-distribute.org/quickstart.html#lay-out-your-project

The structure built will match the following:

| ProjectName/
|---> LICENSE.txt
    | README.txt
    | setup.py
    | projectname/
    | ---> __init__.py

For the sake of best practices, __init__.py is left empty.

'''

#! usr/bin/env python

import sys, os, errno

class NewProject(object):
    '''interface for building a directory with a new project in it'''

    def __init__(self, projectname, directory):

The python style guide recommends underscores to separate words self.project = projectname

I'd call it self.name not self.project. The name project doesn't convey much useful information.

        self.base = check_path(directory)
        #project defaults: I don't want them here as class attributes!
        self.SETUP = {

'README.txt' :
'''This is an unmodified README text file.
It contains a list of changes to the program.
It also contains help for dealing with the application.
''',

'LISCENSE.txt' :
'''Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License
'''
}

I wouldn't mix large docstrings into my python files. I think that makes it rather hard to read.

    def newproject(self):
        '''creates a file structure for a new project at `directory`
        '''
        self.name = self.project.replace(' ', '')

I dislike adding object attirbutes not present in the constructor. It makes it harder to know what attributes an object will have

        self.path = os.sep.join([self.base, self.name])

Use os.path.join for joining paths together. It'll handle a few corner cases that os.sep.join won't.

        sub = self.name.lower()
        subpath = os.sep.join([self.path, sub])

        check_build_path(subpath)
        for filename, content in self.SETUP.items():
            self.buildfile(filename, content, directory=self.path)
        #setup takes arguments, it has its own method
        setup = self.buildsetup()
        self.buildfile('setup.py', setup)
        self.buildfile('__init__.py', directory=subpath)

I'd probably pass a blank content for __init__.py rather then having it be the default. I figure its rate that you want to have a file like that, so making it default doesn't make sense.

    def buildfile(self, name, content="", directory = ""):
        '''opens and creates a new file at `directory` with `contents`'''
        #assumes bad directories have been purified
        if directory == "":
            loc = os.sep.join([self.path, name])
            w = open(loc, 'w')
        else:
            directory = os.sep.join([directory, name])

That's not a directory if you've saved the file name over it. w = open(directory, 'w')

If you use os.path.join, you won't need to handle these two cases separately. w.write(content) w.close()

I recommend you use with statement to make sure the file closes correctly

    def buildsetup(self):
        return '''from distutils.core import setup

setup(
    name='{0}',
    version='0.1dev',
    packages=['{1}',],
    license=open('LISCENSE.txt).read(),
    long_description=open('README.txt').read(),
)
'''.format(self.project, self.name.lower())

Again, I don't like putting huge docstrings in the middle of the code. I recommend having a template folder that you copy over to the target. Just do search/replace on filenames and contents for a few keywords like NAME or MODULE.

def check_path(loc):
    '''recursively check if last character in `loc` is like os.sep character.
    If so, remove.'''
    if loc[-1] == os.sep:
        return check_path(loc[:-1])
    return loc

This is the sorta thing that os.path.join will do for you. I'd also avoid recursion, because its pretty easy to make this a while loop. Also what happens when loc is "/"

def check_build_path(loc):
    d = os.path.normpath(loc)

I discourage single letter variable names whenever I can

    if not os.path.exists(d):
        os.makedirs(d)
    else:
        try:
            os.rmdir(d)
        except OSError as ex:
            if ex.errno == errno.ENOTEMPTY:
                print "Directory specified must be new or empty"
                sys.exit(1)

You should reraise the exception if it was something else.

        #if delete was successful, build the directory
        check_build_path(loc)

This function would be simpler if done like this:

try:
    os.rmdir(d)
except OSError as error:
    if error == ERROR_CODE_FOR_NOT_EXIT:
        pass # OK
    else:
        print "Failed to delete because: ", error
        sys.exit(1)
os.makedirs(d)

I think that conveys the logic better

if __name__ == '__main__':
    if len(sys.argv) != 3:
        print "Usage: string 'Project Name', string '/abs/path'"
        sys.exit(1)
    project = NewProject(sys.argv[1], sys.argv[2])
    project.newproject()
\$\endgroup\$
2
  • \$\begingroup\$ There are 3-4 lines of code with formatting issues. Reading would be much better if fixed. I did myself but as formatting doesn't consume characters the fix didnt get accepted as the system requires 6 char changes. I didnt like/want to add text to others answer, but maybe you do. \$\endgroup\$
    – joaquin
    Dec 26 '11 at 9:45
  • \$\begingroup\$ @joaquin, I'm not sure what issue you are concerned about. Feel free to add text if you want. \$\endgroup\$ Dec 26 '11 at 16:59

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.