FAQ
Is there a forum where one could post a Python script and have it
critiqued by others?

Something like: Y would be more efficent if you did it this way, or
doing it that way could cause problems with X, etc.

Thanks!!!

Search Discussions

  • Derek / nul at Aug 27, 2003 at 11:03 pm
    here will do
    On Wed, 27 Aug 2003 18:33:21 -0400, hokiegal99 wrote:

    Is there a forum where one could post a Python script and have it
    critiqued by others?

    Something like: Y would be more efficent if you did it this way, or
    doing it that way could cause problems with X, etc.

    Thanks!!!
  • Hokiegal99 at Aug 27, 2003 at 11:51 pm

    derek / nul wrote:
    here will do
    OK, I have a few questions about the script at the end of this message:

    Is this script written well? Is it a good design? How could it be made
    better (i.e. write to a file exactly what the scipt did)? I hope to use
    it to recursively find a replace strings in all types of files (binary
    and text).

    Thanks for any ideas, pointers or critique... most of the ideas behind
    the script came from the list.


    #Thanks to comp.lang.python
    import os, string
    print " "
    print "******************************************************"
    print " Three Easy Steps to a Recursive find and Replace "
    print "******************************************************"
    print " "
    x = raw_input("1. Enter the string that you'd like to find: ")
    print " "
    y = raw_input("2. What would you like to replace %s with: " %x)
    print " "
    setpath = raw_input("3. Enter the path where the prgroam should run: ")
    print " "
    for root, dirs, files in os.walk(setpath):
    fname = files
    for fname in files:
    inputFile = file(os.path.join(root,fname), 'r')
    data = inputFile.read()
    inputFile.close()
    search = string.find(data, x)
    if search >=1:
    data = data.replace(x, y)
    outputFile = file(os.path.join(root,fname), 'w')
    outputFile.write(data)
    outputFile.close()
    print "Replacing", x, "with", y, "in", fname
    print " "
    print "**********"
    print " Done "
    print "**********"
    print " "
  • Mackstann at Aug 28, 2003 at 12:22 am
    Looks good, there is one small bug and a couple very minor things that
    basically boil down to style and personal preference.

    -----------------------------------------------------------

    import os # no need for string

    # this can just go in one print with triple quotes
    print """
    ******************************************************
    Three Easy Steps to a Recursive find and Replace
    ******************************************************
    """

    x = raw_input("1. Enter the string that you'd like to find: ")

    # if you just want a newline, you can just do a print by itself
    print

    y = raw_input("2. What would you like to replace %s with: " %x)
    print

    setpath = raw_input("3. Enter the path where the prgroam should run: ")
    print

    for root, dirs, files in os.walk(setpath):
    fname = files # what's this for? fname will just get reassigned on
    # the next line.
    for fname in files:
    inputFile = file(os.path.join(root,fname), 'r')
    data = inputFile.read()
    inputFile.close()
    search = data.find(x) # .find is a method of str, you don't need
    # the string module for it.

    # bug
    #if search >=1: # this won't find it at the beginning of the file.

    if search >= 0: # change to this (find returns -1 when it finds
    # nothing)
    data = data.replace(x, y)
    outputFile = file(os.path.join(root,fname), 'w')
    outputFile.write(data)
    outputFile.close()
    print "Replacing", x, "with", y, "in", fname


    # You could also do something like this to print out the centered
    # messages:

    print "*"*10
    print "Done".center(10)
    print "*"*10

    -----------------------------------------------------------

    HTH,
    --
    m a c k s t a n n mack @ incise.org http://incise.org
    Dentist, n.:
    A Prestidigitator who, putting metal in one's mouth, pulls
    coins out of one's pockets.
    -- Ambrose Bierce, "The Devil's Dictionary"
  • Sean Ross at Aug 28, 2003 at 12:28 am
    "hokiegal99" <hokiegal99 at hotmail.com> wrote in message
    news:3F4D4402.3060209 at hotmail.com...

    Here's a some trivial (mostly cosmetic) changes:

    # multi-line string
    print """
    ******************************************************
    Three Easy Steps to a Recursive Find and Replace
    ******************************************************
    """
    text = raw_input("1. Enter the string that you'd like to find: ")
    replacement = raw_input("\n2. What would you like to replace '%s' with:
    "%text)
    path = raw_input("\n3. Enter the path where the program should run: ")
    print
    # " " is not required.

    for root, dirs, files in os.walk(path):
    for fname in files:
    filename = os.path.join(root,fname)
    fd = file(filename, 'r')
    data = fd.read()
    fd.close()
    if string.find(data, text) >=1:
    data = data.replace(text, replacement)
    fd = file(filename, 'w')
    fd.write(data)
    fd.close()
    print "Replacing '%s' with '%s' in '%s'" % (text, replacement,
    fname)

    print """
    **********
    Done
    **********
    """

    # Note: I haven't tested the changes.

    Hope that's useful,
    Sean
  • Hokiegal99 at Aug 28, 2003 at 2:38 am

    Sean Ross wrote:
    "hokiegal99" <hokiegal99 at hotmail.com> wrote in message
    news:3F4D4402.3060209 at hotmail.com...

    Here's a some trivial (mostly cosmetic) changes:

    # multi-line string
    print """
    ******************************************************
    Three Easy Steps to a Recursive Find and Replace
    ******************************************************
    """
    text = raw_input("1. Enter the string that you'd like to find: ")
    replacement = raw_input("\n2. What would you like to replace '%s' with:
    "%text)
    path = raw_input("\n3. Enter the path where the program should run: ")
    print
    # " " is not required.

    for root, dirs, files in os.walk(path):
    for fname in files:
    filename = os.path.join(root,fname)
    fd = file(filename, 'r')
    data = fd.read()
    fd.close()
    if string.find(data, text) >=1:
    data = data.replace(text, replacement)
    fd = file(filename, 'w')
    fd.write(data)
    fd.close()
    print "Replacing '%s' with '%s' in '%s'" % (text, replacement,
    fname)

    print """
    **********
    Done
    **********
    """

    # Note: I haven't tested the changes.

    Hope that's useful,
    Sean
    Yes, that's useful. The newlines saves space as does the print """
    command. Thanks!!!
  • Hokiegal99 at Aug 28, 2003 at 12:54 am
    Something odd that I've discovered about this script is that it won't
    replace strings at the far left side of a text file *unless* the string
    has one or more spaces before it. For example:

    THIS (won't be replace with THAT)
    THIS (will be replaced with THAT)
    THIS (will be replaced with THAT)
    THIS (will be replaced with THAT)

    #Thanks to comp.lang.python
    import os, string
    print " "
    print "******************************************************"
    print " Three Easy Steps to a Recursive find and Replace "
    print "******************************************************"
    print " "
    x = raw_input("1. Enter the string that you'd like to find: ")
    print " "
    y = raw_input("2. What would you like to replace %s with: " %x)
    print " "
    setpath = raw_input("3. Enter the path where the prgroam should run: ")
    print " "
    for root, dirs, files in os.walk(setpath):
    fname = files
    for fname in files:
    inputFile = file(os.path.join(root,fname), 'r')
    data = inputFile.read()
    inputFile.close()
    search = string.find(data, x)
    if search >=1:
    data = data.replace(x, y)
    outputFile = file(os.path.join(root,fname), 'w')
    outputFile.write(data)
    outputFile.close()
    print "Replacing", x, "with", y, "in", fname
    print " "
    print "**********"
    print " Done "
    print "**********"
    print " "
  • Mackstann at Aug 28, 2003 at 1:03 am

    On Wed, Aug 27, 2003 at 08:54:06PM -0400, hokiegal99 wrote:
    Something odd that I've discovered about this script is that it won't
    replace strings at the far left side of a text file *unless* the string
    has one or more spaces before it. For example:

    THIS (won't be replace with THAT)
    THIS (will be replaced with THAT)
    THIS (will be replaced with THAT)
    THIS (will be replaced with THAT)
    See my reply :)

    --
    m a c k s t a n n mack @ incise.org http://incise.org
    The church is near but the road is icy; the bar is far away but I will
    walk carefully.
    -- Russian Proverb
  • Hokiegal99 at Aug 28, 2003 at 2:35 am
    Yes, I got it... thanks for pointing out the flaw in my logic! And for
    the other tips as well. I especially liked the one about string. I had
    no idea that I didn't need to use that. Thanks again!!!

    mackstann wrote:
    On Wed, Aug 27, 2003 at 08:54:06PM -0400, hokiegal99 wrote:

    Something odd that I've discovered about this script is that it won't
    replace strings at the far left side of a text file *unless* the string
    has one or more spaces before it. For example:

    THIS (won't be replace with THAT)
    THIS (will be replaced with THAT)
    THIS (will be replaced with THAT)
    THIS (will be replaced with THAT)

    See my reply :)
  • Terry Reedy at Aug 28, 2003 at 1:22 am
    "hokiegal99" <hokiegal99 at hotmail.com> wrote in message
    news:3F4D4402.3060209 at hotmail.com...
    Is this script written well? Is it a good design? How could it be made
    better (i.e. write to a file exactly what the scipt did)? I hope to use
    it to recursively find a replace strings in all types of files (binary
    and text).
    My main answer is your answer to the following: does it operate
    correctly? does it produce output within the bounds of what you
    consider acceptible? In particular, does it pass your test case(s)?
    and are your test case(s) reasonably representative of the domain of
    application?
    Thanks for any ideas, pointers or critique... most of the ideas behind
    the script came from the list.


    #Thanks to comp.lang.python
    import os, string
    print " "
    print "******************************************************"
    print " Three Easy Steps to a Recursive find and Replace "
    print "******************************************************"
    I would lazily print one multiline string.
    print " "
    x = raw_input("1. Enter the string that you'd like to find: ")
    I would delete the print and add '\n' to the front of the prompt
    print " "
    y = raw_input("2. What would you like to replace %s with: " %x)
    print " "
    setpath = raw_input("3. Enter the path where the prgroam should run: ")
    print " "
    for root, dirs, files in os.walk(setpath):
    fname = files
    for fname in files:
    inputFile = file(os.path.join(root,fname), 'r')
    data = inputFile.read()
    inputFile.close()
    search = string.find(data, x)
    if search >=1:
    data = data.replace(x, y)
    outputFile = file(os.path.join(root,fname), 'w')
    outputFile.write(data)
    outputFile.close()
    print "Replacing", x, "with", y, "in", fname
    print " "
    print "**********"
    print " Done "
    print "**********"
    Terry J. Reedy
  • Hokiegal99 at Aug 28, 2003 at 2:37 am

    Terry Reedy wrote:

    My main answer is your answer to the following: does it operate
    correctly? does it produce output within the bounds of what you
    consider acceptible? In particular, does it pass your test case(s)?
    and are your test case(s) reasonably representative of the domain of
    application?
    Well it works now, mackstan pointed out a flaw in my logic that caused
    the script to miss stings if they occured at the very front of a file.
    Thanks!!!
  • Bruno Desthuilliers at Aug 28, 2003 at 6:30 am

    hokiegal99 wrote:
    derek / nul wrote:
    here will do

    OK, I have a few questions about the script at the end of this message:

    Is this script written well? Is it a good design? How could it be made
    better (i.e. write to a file exactly what the scipt did)? I hope to use
    it to recursively find a replace strings in all types of files (binary
    and text).

    Thanks for any ideas, pointers or critique... most of the ideas behind
    the script came from the list.

    Err... Is this my version of Python having troubles, or could it be
    possible that nobody actually took time to *test* that script ?
    There is a real problem on line 15:
    for root, dirs, files in os.walk(setpath):
    on Python 2.2.2, here is what I get :

    [laotseu at localhost dev]$ python rfp.py
    (snip)
    Traceback (most recent call last):
    File "rfp.py", line 15, in ?
    for root, dirs, files in os.walk(setpath):
    AttributeError: 'module' object has no attribute 'walk'
    [3]+ Done emacs testrfp
    [laotseu at localhost dev]$


    Of course, 'walk' is in os.path, not in os.

    Python 2.2.2 (#2, Feb 5 2003, 10:40:08)
    [GCC 3.2.1 (Mandrake Linux 9.1 3.2.1-5mdk)] on linux-i386
    Type "help", "copyright", "credits" or "license" for more information.
    import os
    os.walk
    Traceback (most recent call last):
    File "<stdin>", line 1, in ?
    AttributeError: 'module' object has no attribute 'walk'

    Now a another problem on the same line :
    for root, dirs, files in os.path.walk(setpath):
    ... print root, dirs, files
    ...
    Traceback (most recent call last):
    File "<stdin>", line 1, in ?
    TypeError: walk() takes exactly 3 arguments (1 given)

    Of course, this is not the syntax nor the way to use os.path.walk

    I checked the os module for a function with similar syntax and usage,
    and could not find one. os.listdir would have been a candidate, but it
    does not recurse, and do not return a (root, dirs, files) tuple but a
    list of filenames.

    So what ?

    Bruno
  • Peter Otten at Aug 28, 2003 at 6:41 am

    Bruno Desthuilliers wrote:

    Err... Is this my version of Python having troubles, or could it be
    possible that nobody actually took time to *test* that script ?
    Python 2.3 (#1, Jul 30 2003, 11:19:43)
    [GCC 3.2] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    import os
    dir(os.walk)
    ['__call__', '__class__', '__delattr__', '__dict__', '__doc__', '__get__',
    '__getattribute__', '__hash__', '__init__', '__module__', '__name__',
    '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__',
    '__str__', 'func_closure', 'func_code', 'func_defaults', 'func_dict',
    'func_doc', 'func_globals', 'func_name']
    >>>

    It's time to upgrade :-)

    Peter
  • Bruno Desthuilliers at Aug 28, 2003 at 8:32 am

    Peter Otten wrote:
    Bruno Desthuilliers wrote:

    Err... Is this my version of Python having troubles, or could it be
    possible that nobody actually took time to *test* that script ?

    Python 2.3 (#1, Jul 30 2003, 11:19:43)
    [GCC 3.2] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    import os
    dir(os.walk)
    ['__call__', '__class__', '__delattr__', '__dict__', '__doc__', '__get__',
    '__getattribute__', '__hash__', '__init__', '__module__', '__name__',
    '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__',
    '__str__', 'func_closure', 'func_code', 'func_defaults', 'func_dict',
    'func_doc', 'func_globals', 'func_name']


    It's time to upgrade :-)
    Seems like !-)
    I could not believe no one has spotted such a bug.

    Thanks everyone
    Bruno
  • Seo Sanghyeon at Aug 28, 2003 at 6:43 am
    (hokiengal99 posted a script for critique.)

    Bruno Desthuilliers wrote:
    Err... Is this my version of Python having troubles, or could it be
    possible that nobody actually took time to *test* that script?
    On Python 2.2.2, here is what I get:
    (here follows what he got)

    Neither. The answer is that people on this thread used Python 2.3.
    Check the documentation here: http://www.python.org/doc/lib/os-file-dir.html
    (os.walk() is documented at the end.)

    os.walk() is introduced because using os.path.walk() can be hard.
    It's new in 2.3.

    Seo Sanghyeon
  • Peter Otten at Aug 28, 2003 at 9:03 am

    hokiegal99 wrote:

    Thanks for any ideas, pointers or critique... most of the ideas behind
    the script came from the list.
    Choose variable names that make their intended purpose clear, e.g.
    searchToken/replaceToken instead of x/y.
    search = string.find(data, x)
    if search >=1:
    This is wrong as others have already explained. I recommend:

    if searchToken in data:
    # perform replacement

    (I did it with .find() till yesterday, but no more! I like that Python
    minimizes the need of artificial indices:-)

    Factor out reusable code, e.g:

    def replaceTokenInTree(rootFolder, searchToken, replaceToken,
    filterFunc=None)
    def replaceTokenInFile(filename, searchToken, replaceToken, backupFile=None)

    This will pay off as your scripts grow (or your script grows:-), so make it
    a habit as soon as possible.


    Some observations that are not directly code-related:

    You can change - and possibly corrupt - *many* files in one run of you
    script. So you might add a confirmation prompt repeating searchToken,
    replaceToken and the root directory (resolved to an absolute path) and -
    somewhat more ambitious - a means to generate backups.

    When replacement fails on one file, e. g. due to access rights, you could
    catch the exception and prompt the user, if he wants to continue or abort
    the script.

    Chances are that there are some files in the tree that you do not want to
    process, so you should add some basic filtering. The fnmatch module might
    be helpful.


    Last but most important:

    Take time to build a test file tree, and include all special cases you can
    think of (including those you consider irrelevant) e. g. files
    starting/ending with search token, zerolength file, file with readonly
    access.

    Peter
  • Hokieghal99 at Aug 28, 2003 at 1:23 pm

    Peter Otten wrote:
    Factor out reusable code, e.g:

    def replaceTokenInTree(rootFolder, searchToken, replaceToken,
    filterFunc=None)
    def replaceTokenInFile(filename, searchToken, replaceToken, backupFile=None)

    This will pay off as your scripts grow (or your script grows:-), so make it
    a habit as soon as possible.
    I don't fully understand how functions work yet. This is something that
    I'm studying in my spare time. I dislike using something when I don't
    fully understand how it works, thus my scripts are really very simple
    right now. That's one reason I like Python so well, it has milk for
    babies like me and meat for adults like you. It's useful and productive
    for both of us at different stages in our knowledge.

    Some observations that are not directly code-related:

    You can change - and possibly corrupt - *many* files in one run of you
    script. So you might add a confirmation prompt repeating searchToken,
    replaceToken and the root directory (resolved to an absolute path) and -
    somewhat more ambitious - a means to generate backups.
    This is very good. I will add the confirmation check soon. Thank you for
    the suggestion. The backup idea is good too, not 100% necessary, but
    highly desirable.
    Last but most important:

    Take time to build a test file tree, and include all special cases you can
    think of (including those you consider irrelevant) e. g. files
    starting/ending with search token, zerolength file, file with readonly
    access.
    Yes, I do this a lot, but it's just me and I'm limited to my world view
    of things. This is why I like this forum, you guys point things out that
    I miss entirely.

    Thanks!!!
  • Ganesan R at Aug 28, 2003 at 9:18 am

    "hokiegal99" == hokiegal99 <hokiegal99 at hotmail.com> writes:
    Is this script written well? Is it a good design? How could it be made
    better (i.e. write to a file exactly what the scipt did)? I hope to
    use it to recursively find a replace strings in all types of files
    (binary and text).
    Thanks for any ideas, pointers or critique... most of the ideas behind
    the script came from the list.
    Others have done an excellent job of telling you the problems of critiquing
    your code. I'll try to address one point that others haven't covered
    already.
    print " "
    print "******************************************************"
    print " Three Easy Steps to a Recursive find and Replace "
    print "******************************************************"
    print " "
    x = raw_input("1. Enter the string that you'd like to find: ")
    print " "
    y = raw_input("2. What would you like to replace %s with: " %x)
    print " "
    setpath = raw_input("3. Enter the path where the prgroam should run: ")
    What you've done is not wrong. But it's simpler to replace all that with

    import sys
    x = sys.argv[1]
    y = sys.argv[2]
    setpath = sys.argv[3]

    so that you can run it as "python myscript.py <search> <replace> <path>".
    This way you can easily call your script from another program and
    makes it much more useful. You can do something like

    if len(sys.argv) >= 1:
    x = sys.argv[1]
    y = sys.argv[2]
    setpath = sys.argv[3]
    else:
    raw_input(...)
    ...

    and get the best of both worlds.

    Ganesan

    --
    Ganesan R
  • Hokieghal99 at Aug 28, 2003 at 1:30 pm

    Ganesan R wrote:
    Others have done an excellent job of telling you the problems of critiquing
    your code. I'll try to address one point that others haven't covered
    already.

    What you've done is not wrong. But it's simpler to replace all that with

    import sys
    x = sys.argv[1]
    y = sys.argv[2]
    setpath = sys.argv[3]

    so that you can run it as "python myscript.py <search> <replace> <path>".
    This way you can easily call your script from another program and
    makes it much more useful. You can do something like

    if len(sys.argv) >= 1:
    x = sys.argv[1]
    y = sys.argv[2]
    setpath = sys.argv[3]
    else:
    raw_input(...)
    ...
    I don't understand this approach to developing a script. I think this is
    more of a true program than a script... it's much more complex. My
    background is shell scripting so I'm limited to what I've learned from
    that. Python is much deeper than that, but I'm learning. Thank you for
    the advice.
  • Gerrit Holl at Aug 28, 2003 at 9:47 am

    hokiegal99 wrote:
    print " "
    The " " is unnecesary. The following statement does the same:

    print

    To the eye, there is no difference between th two.
    print "******************************************************"
    print " Three Easy Steps to a Recursive find and Replace "
    print "******************************************************"
    I usually create a string first and then print it:

    intro = """
    ========================
    etc
    ========================
    """

    print intro

    This makes it more comfortable to change it.

    Gerrit.

    --
    181. If a father devote a temple-maid or temple-virgin to God and give
    her no present: if then the father die, she shall receive the third of a
    child's portion from the inheritance of her father's house, and enjoy its
    usufruct so long as she lives. Her estate belongs to her brothers.
    -- 1780 BC, Hammurabi, Code of Law
    --
    Asperger Syndroom - een persoonlijke benadering:
    http://people.nl.linux.org/~gerrit/
    Het zijn tijden om je zelf met politiek te bemoeien:
    http://www.sp.nl/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppython-list @
categoriespython
postedAug 27, '03 at 10:33p
activeAug 28, '03 at 1:30p
posts20
users10
websitepython.org

People

Translate

site design / logo © 2022 Grokbase