Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.python > #109801 > unrolled thread

i'm a python newbie & wrote my first script, can someone critique it?

Back to article view | Back to comp.lang.python

Contents

  i'm a python newbie & wrote my first script, can someone critique it? mad scientist jr <mad.scientist.jr@gmail.com> - 2016-06-10 15:52 -0700
    Re: i'm a python newbie & wrote my first script, can someone critique it? Christopher Reimer <christopher_reimer@icloud.com> - 2016-06-10 16:05 -0700
    Re: i'm a python newbie & wrote my first script, can someone critique it? Marc Brooks <marcwbrooks@gmail.com> - 2016-06-10 20:51 -0400
    Re: i'm a python newbie & wrote my first script, can someone critique it? Joel Goldstick <joel.goldstick@gmail.com> - 2016-06-10 21:02 -0400
    Re: i'm a python newbie & wrote my first script, can someone critique it? Larry Hudson <orgnut@yahoo.com> - 2016-06-10 21:00 -0700
    Re: i'm a python newbie & wrote my first script, can someone critique it? Matt Wheeler <m@funkyhat.org> - 2016-06-11 04:28 +0000
      Re: i'm a python newbie & wrote my first script, can someone critique it? mad scientist jr <mad.scientist.jr@gmail.com> - 2016-06-11 10:59 -0700
        Re: i'm a python newbie & wrote my first script, can someone critique it? mad scientist jr <mad.scientist.jr@gmail.com> - 2016-06-11 11:14 -0700
          Re: i'm a python newbie & wrote my first script, can someone critique it? Marc Brooks <marcwbrooks@gmail.com> - 2016-06-11 18:18 +0000
        Re: i'm a python newbie & wrote my first script, can someone critique it? MRAB <python@mrabarnett.plus.com> - 2016-06-11 19:41 +0100
          Re: i'm a python newbie & wrote my first script, can someone critique it? mad scientist jr <mad.scientist.jr@gmail.com> - 2016-06-12 09:04 -0700

#109801 — i'm a python newbie & wrote my first script, can someone critique it?

Is this group appropriate for that kind of thing? 
(If not sorry for posting this here.)

So I wanted to start learning Python, and there is soooo much information online, which is a little overwhelming. I really learn best from doing, especially if it's something actually useful. I needed to create a bunch of empty folders, so I figured it was a good exercise to start learning Python. 
Now that it's done, I am wondering what kind of things I could do better. 
Here is the code: 

# WELCOME TO MY FIRST PYTHON SCRIPT!
# FIRST OF ALL, IT ***WORKS***!!! YAY!
# I AM A VBA AND JAVASCRIPT PROGRAMMER, 
# SO IT IS PROBABLY NOT VERY "PYTHONIC", 
# SO PLEASE FEEL FREE TO TEAR THE SCRIPT A NEW ONE
# AFTER YOU ARE SHOCKED BY THIS BAD CODE, 
# ALL I ASK IS THAT IF YOU THINK SOMETHING IS BAD, 
# PLEASE POST AN EXAMPLE OF THE "CORRECT" WAY OF DOING IT
# COMING FROM VBA, I KNOW A LITTLE OOP,
# BUT NOT C++ C# JAVA STUFF LIKE "INTERFACES" AND "ABSTRACT BASE CLASSES"
# SO IF YOU GET INTO SUCH TERMINOLOGY MY EYES MIGHT START TO GLAZE OVER
# UNLESS YOU CARE TO EXPLAIN THAT TOO

############################################################################################################################################################
# GLOBAL VALUES
sForPythonVersion="3"
#sFolderPathTemplate = "c:\\myscripts\\MP3 Disc <iCount/>"
sFolderPathTemplate = "c:\\temp\\MP3 Disc <iCount/>"
iFromCount = 1
iToCount = 250
iCountWidth = 3

################################################################################################################################################################
# SUPPORT FUNCTIONS

# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
def is_string(myVar): # is_string IS MORE READABLE THAN isinstance (PLAIN ENGLISH!)
    #PYTHON 3 IS NOT LIKING THIS: return ( isinstance(myVar, str) or isinstance(myVar, unicode) )
    #PYTHON 3 IS NOT LIKING THIS: return isinstance(myVar, basestr)
    return isinstance(myVar, str)

# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
# THIS IS SOME SAMPLE FUNCTION FROM A BOOK I AM READING "PYTHON IN EASY STEPS"
def strip_one_space(s):
    if s.endswith(" "): s = s[:-1]
    if s.startswith(" "): s = s[1:]
    return s

# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
def get_exact_python_version():
    import sys
    sVersion = ".".join(map(str, sys.version_info[:3]))
    sVersion = sVersion.strip()
    return sVersion

# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
# TO DO: RETURN TO THE LEFT OF FIRST "."
def get_python_version():
    sVersion = get_exact_python_version()
    return sVersion[0:1]

# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
# CHECK PYTHON VERSION, IF IT'S WRONG THEN GRACEFULLY EXIT BEFORE IT BLOWS UP
# (DAMN, PYTHON 2.x STILL COMPLAINS WITH SYNTAX ERRORS!!)
# MAYBE THIS COULD STILL BE USEFUL FOR CHECKING THE SUB-VERSION, IN THAT CASE
# TO DO: MORE GRANULAR CHECK, EG IF VERSION >= 3.5.0
def exit_if_wrong_python_version(sRightVersion):
    import os
    sCurrentVersion = get_python_version()
    if (sCurrentVersion != sRightVersion):
        print("" +
              "Wrong Python version (" +
              sCurrentVersion +
              "), this script should be run using Python " +
              sRightVersion +
              ".x. Exiting..."
              )
        os._exit(0)

# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
def get_script_filename():
    import os
    return os.path.basename(__file__)

# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
def get_timestamp():
    import datetime
    return datetime.datetime.strftime(datetime.datetime.now(), '%Y-%m-%d %H:%M:%S')
    
# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

def create_folder(sPath):
    import os
    import errno
    #if not os.path.exists(directory):
    #    os.makedirs(directory)
    try:
        os.makedirs(sPath, exist_ok=True)
    except OSError as exception:
        #if exception.errno != errno.EEXIST:
        print("ERROR #" + str(exception.errno) + "IN makedirs")
        raise
    
# //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

def create_folders(sFolderPathTemplate:str="", iFromCount:int=1, iToCount:int=0, iCountWidth:int=0):
    # MAKE SURE TEMPLATE'S A STRING. OH, IS THIS NOT "PYTHONIC"? WELL IT'S MORE READABLE
    if is_string(sFolderPathTemplate) == False:
        iFromCount = 1; iToCount = 0;
    
    sName = ""
    sCount = ""
    iLoop = iFromCount
    while (iLoop <= iToCount) and (len(sFolderPathTemplate) > 0):
        sCount = "{0}".format(iLoop)
        if (iCountWidth > 0):
            sCount = sCount.zfill(iCountWidth)
        sName = sFolderPathTemplate.replace("<iCount/>", sCount)
        create_folder(sName)
        iLoop = iLoop + 1

############################################################################################################################################################
# MAIN LOGIC

def main():
    # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
    # MAKE SURE PYTHON VERSION IS CORRECT
    exit_if_wrong_python_version(sForPythonVersion)
    print("PYTHON VERSION (" + get_exact_python_version() + ") MATCHES REQUIRED VERSION (" + sForPythonVersion + ")")
    #print("get_python_version       returns \"" + get_python_version()       + "\"")
    #print("get_exact_python_version returns \"" + get_exact_python_version() + "\"")

    # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
    # PRINT START TIMESTAMP
    print("" + get_timestamp() + " " + get_script_filename() + " STARTED.")

    # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
    # DO WHAT WE CAME TO DO
    # IT'S PROBABLY BAD FORM TO REFERENCE GLOBAL VARIABLES INSIDE THE SCOPE OF A FUNCTION?
    # BUT I WANT TO MAKE IT EASY TO CONFIGURE THE SCRIPT JUST BY CHANGING A COUPLE OF LINES
    # AT THE TOP, WITHOUT HAVING TO SEARCH THROUGH THE CODE
    create_folders(
        sFolderPathTemplate,
        iFromCount,
        iToCount,
        iCountWidth)

    # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
    # NOW FINISH
    print("" + get_timestamp() + " " + get_script_filename() + " FINISHED.")
    #import os
    #os._exit(0)

############################################################################################################################################################

main()

[toc] | [next] | [standalone]

#109804

Sent from my iPhone

> On Jun 10, 2016, at 3:52 PM, mad scientist jr <mad.scientist.jr@gmail.com> wrote:
> . 
> Now that it's done, I am wondering what kind of things I could do better.

This is Python, not BASIC. Lay off on the CAP LOCK key and delete all the comments and separation blocks. No one is going to find your code in all that noise.

Chris R.

[toc] | [prev] | [next] | [standalone]

#109806

The structure of your program is really not that Pythonic.  I'd recommend
you take a look at PEP 8.

https://www.python.org/dev/peps/pep-0008/

It's not perfect, but it's a good start to get a feel for how to structure
your Python work.

Marc

On Fri, Jun 10, 2016 at 7:05 PM, Christopher Reimer <
christopher_reimer@icloud.com> wrote:

>
>
> Sent from my iPhone
>
> > On Jun 10, 2016, at 3:52 PM, mad scientist jr <
> mad.scientist.jr@gmail.com> wrote:
> > .
> > Now that it's done, I am wondering what kind of things I could do better.
>
> This is Python, not BASIC. Lay off on the CAP LOCK key and delete all the
> comments and separation blocks. No one is going to find your code in all
> that noise.
>
> Chris R.
> --
> https://mail.python.org/mailman/listinfo/python-list
>

[toc] | [prev] | [next] | [standalone]

#109809

On Fri, Jun 10, 2016 at 6:52 PM, mad scientist jr
<mad.scientist.jr@gmail.com> wrote:
> Is this group appropriate for that kind of thing?
> (If not sorry for posting this here.)
>
> So I wanted to start learning Python, and there is soooo much information online, which is a little overwhelming. I really learn best from doing, especially if it's something actually useful. I needed to create a bunch of empty folders, so I figured it was a good exercise to start learning Python.
> Now that it's done, I am wondering what kind of things I could do better.
> Here is the code:
>
> # WELCOME TO MY FIRST PYTHON SCRIPT!
> # FIRST OF ALL, IT ***WORKS***!!! YAY!
> # I AM A VBA AND JAVASCRIPT PROGRAMMER,
> # SO IT IS PROBABLY NOT VERY "PYTHONIC",
> # SO PLEASE FEEL FREE TO TEAR THE SCRIPT A NEW ONE
> # AFTER YOU ARE SHOCKED BY THIS BAD CODE,
> # ALL I ASK IS THAT IF YOU THINK SOMETHING IS BAD,
> # PLEASE POST AN EXAMPLE OF THE "CORRECT" WAY OF DOING IT
> # COMING FROM VBA, I KNOW A LITTLE OOP,
> # BUT NOT C++ C# JAVA STUFF LIKE "INTERFACES" AND "ABSTRACT BASE CLASSES"
> # SO IF YOU GET INTO SUCH TERMINOLOGY MY EYES MIGHT START TO GLAZE OVER
> # UNLESS YOU CARE TO EXPLAIN THAT TOO
>
>
Ditch the uppercase comments.
 ############################################################################################################################################################
> # GLOBAL VALUES
> sForPythonVersion="3"
> #sFolderPathTemplate = "c:\\myscripts\\MP3 Disc <iCount/>"
> sFolderPathTemplate = "c:\\temp\\MP3 Disc <iCount/>"
> iFromCount = 1
> iToCount = 250
> iCountWidth = 3
>
Python has guidelines for naming things.  Don't use camel case.  Use
lower case and put underscores between words.  Globals are a bad idea.
If you don't know why, they are even a worse idea
> ################################################################################################################################################################
> # SUPPORT FUNCTIONS
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def is_string(myVar): # is_string IS MORE READABLE THAN isinstance (PLAIN ENGLISH!)
>     #PYTHON 3 IS NOT LIKING THIS: return ( isinstance(myVar, str) or isinstance(myVar, unicode) )
>     #PYTHON 3 IS NOT LIKING THIS: return isinstance(myVar, basestr)
>     return isinstance(myVar, str)

I'm not sure why you want to test for type.
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # THIS IS SOME SAMPLE FUNCTION FROM A BOOK I AM READING "PYTHON IN EASY STEPS"
> def strip_one_space(s):
>     if s.endswith(" "): s = s[:-1]
>     if s.startswith(" "): s = s[1:]
>     return s

This may be from some book, but it could be done:
s.strip()


>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_exact_python_version():
>     import sys
>     sVersion = ".".join(map(str, sys.version_info[:3]))
>     sVersion = sVersion.strip()
>     return sVersion
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # TO DO: RETURN TO THE LEFT OF FIRST "."
> def get_python_version():
>     sVersion = get_exact_python_version()
>     return sVersion[0:1]
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # CHECK PYTHON VERSION, IF IT'S WRONG THEN GRACEFULLY EXIT BEFORE IT BLOWS UP
> # (DAMN, PYTHON 2.x STILL COMPLAINS WITH SYNTAX ERRORS!!)
> # MAYBE THIS COULD STILL BE USEFUL FOR CHECKING THE SUB-VERSION, IN THAT CASE
> # TO DO: MORE GRANULAR CHECK, EG IF VERSION >= 3.5.0
> def exit_if_wrong_python_version(sRightVersion):
>     import os
>     sCurrentVersion = get_python_version()
>     if (sCurrentVersion != sRightVersion):
>         print("" +
>               "Wrong Python version (" +
>               sCurrentVersion +
>               "), this script should be run using Python " +
>               sRightVersion +
>               ".x. Exiting..."
>               )
>         os._exit(0)
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_script_filename():
>     import os
>     return os.path.basename(__file__)
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_timestamp():
>     import datetime
>     return datetime.datetime.strftime(datetime.datetime.now(), '%Y-%m-%d %H:%M:%S')
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> def create_folder(sPath):
>     import os
>     import errno
>     #if not os.path.exists(directory):
>     #    os.makedirs(directory)
>     try:
>         os.makedirs(sPath, exist_ok=True)
>     except OSError as exception:
>         #if exception.errno != errno.EEXIST:
>         print("ERROR #" + str(exception.errno) + "IN makedirs")
>         raise
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> def create_folders(sFolderPathTemplate:str="", iFromCount:int=1, iToCount:int=0, iCountWidth:int=0):
>     # MAKE SURE TEMPLATE'S A STRING. OH, IS THIS NOT "PYTHONIC"? WELL IT'S MORE READABLE
>     if is_string(sFolderPathTemplate) == False:
>         iFromCount = 1; iToCount = 0;
>
>     sName = ""
>     sCount = ""
>     iLoop = iFromCount
>     while (iLoop <= iToCount) and (len(sFolderPathTemplate) > 0):
>         sCount = "{0}".format(iLoop)
>         if (iCountWidth > 0):
>             sCount = sCount.zfill(iCountWidth)
>         sName = sFolderPathTemplate.replace("<iCount/>", sCount)
>         create_folder(sName)
>         iLoop = iLoop + 1
>
> ############################################################################################################################################################
> # MAIN LOGIC
>
> def main():
>     # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # MAKE SURE PYTHON VERSION IS CORRECT
>     exit_if_wrong_python_version(sForPythonVersion)
>     print("PYTHON VERSION (" + get_exact_python_version() + ") MATCHES REQUIRED VERSION (" + sForPythonVersion + ")")
>     #print("get_python_version       returns \"" + get_python_version()       + "\"")
>     #print("get_exact_python_version returns \"" + get_exact_python_version() + "\"")
>
>     # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # PRINT START TIMESTAMP
>     print("" + get_timestamp() + " " + get_script_filename() + " STARTED.")
>
>     # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # DO WHAT WE CAME TO DO
>     # IT'S PROBABLY BAD FORM TO REFERENCE GLOBAL VARIABLES INSIDE THE SCOPE OF A FUNCTION?
>     # BUT I WANT TO MAKE IT EASY TO CONFIGURE THE SCRIPT JUST BY CHANGING A COUPLE OF LINES
>     # AT THE TOP, WITHOUT HAVING TO SEARCH THROUGH THE CODE
>     create_folders(
>         sFolderPathTemplate,
>         iFromCount,
>         iToCount,
>         iCountWidth)
>
>     # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # NOW FINISH
>     print("" + get_timestamp() + " " + get_script_filename() + " FINISHED.")
>     #import os
>     #os._exit(0)
>
> ############################################################################################################################################################
>
> main()
> --
> https://mail.python.org/mailman/listinfo/python-list

I gave up.  I'm happy you want to learn python, and have such
enthusiasm, but you should practice some more
-- 
Joel Goldstick
http://joelgoldstick.com/blog
http://cc-baseballstats.info/stats/birthdays

[toc] | [prev] | [next] | [standalone]

#109812

On 06/10/2016 03:52 PM, mad scientist jr wrote:
> Is this group appropriate for that kind of thing?
> (If not sorry for posting this here.)
>
> So I wanted to start learning Python, and there is soooo much information online, which is a little overwhelming. I really learn best from doing, especially if it's something actually useful. I needed to create a bunch of empty folders, so I figured it was a good exercise to start learning Python.
> Now that it's done, I am wondering what kind of things I could do better.
> Here is the code:
>
It is FAR too complicated -- it's BASIC written Python.  Python is MUCH easier.

First a couple of general comments...

Drop the Hungarian notation!!  Python uses dynamic typing.  Variables do NOT have a type, the 
data they hold have types, but not the variable itself.  ANY variable can hod ANY data type at 
ANY time.  And Python uses duck typing -- get used to it.

Generally you should put all the imports at the beginning of the program, NOT in each function. 
  A possible exception could be if an import is only used in one function.

> ############################################################################################################################################################
> # GLOBAL VALUES
> sForPythonVersion="3"
> #sFolderPathTemplate = "c:\\myscripts\\MP3 Disc <iCount/>"
> sFolderPathTemplate = "c:\\temp\\MP3 Disc <iCount/>"
> iFromCount = 1
> iToCount = 250
> iCountWidth = 3
>
Drop the <iCount/> from the template string.  (More on this below.)

> ################################################################################################################################################################
> # SUPPORT FUNCTIONS
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def is_string(myVar): # is_string IS MORE READABLE THAN isinstance (PLAIN ENGLISH!)
>      #PYTHON 3 IS NOT LIKING THIS: return ( isinstance(myVar, str) or isinstance(myVar, unicode) )
>      #PYTHON 3 IS NOT LIKING THIS: return isinstance(myVar, basestr)
>      return isinstance(myVar, str)
>
Not necessary.

> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # THIS IS SOME SAMPLE FUNCTION FROM A BOOK I AM READING "PYTHON IN EASY STEPS"
> def strip_one_space(s):
>      if s.endswith(" "): s = s[:-1]
>      if s.startswith(" "): s = s[1:]
>      return s
>
???  Is this used anyplace?  Delete it.

> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_exact_python_version():
>      import sys
>      sVersion = ".".join(map(str, sys.version_info[:3]))
>      sVersion = sVersion.strip()
>      return sVersion
>
sys.version_info[:3] by itself gives a three-element tuple.  Probably easier to use than the 
string version.

A couple alternatives:
sys.version[:5] or perhaps a bit safer -- sys.version.split()[0] both directly give you the 
string version.  (Note this uses version not version_info.)

> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # TO DO: RETURN TO THE LEFT OF FIRST "."
> def get_python_version():
>      sVersion = get_exact_python_version()
>      return sVersion[0:1]
>
Probably unnecessary as a function.  A simple sVersion[0] in-line might be easier.

> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # CHECK PYTHON VERSION, IF IT'S WRONG THEN GRACEFULLY EXIT BEFORE IT BLOWS UP
> # (DAMN, PYTHON 2.x STILL COMPLAINS WITH SYNTAX ERRORS!!)
> # MAYBE THIS COULD STILL BE USEFUL FOR CHECKING THE SUB-VERSION, IN THAT CASE
> # TO DO: MORE GRANULAR CHECK, EG IF VERSION >= 3.5.0
> def exit_if_wrong_python_version(sRightVersion):
>      import os
>      sCurrentVersion = get_python_version()
>      if (sCurrentVersion != sRightVersion):
>          print("" +
>                "Wrong Python version (" +
>                sCurrentVersion +
>                "), this script should be run using Python " +
>                sRightVersion +
>                ".x. Exiting..."
>                )
>          os._exit(0)
>
Get used to Python string formatting...
print("Wrong Python version ({}), this script should be run using "
         "Python {}.x,  Exiting...".format(sCurrentVersion, sRightVersion))

Notice how I split the long single-line string into two shorter strings on two lines relying on 
the automatic string concatenation in Python.  "string1 " "string2" becomes "string1 string2". 
Any whitespace (spaces, tabs, newlines) are ignored and the two strings are stuck together.

Also the common use is sys.exit() instead of os._exit().

> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_script_filename():
>      import os
>      return os.path.basename(__file__)
>
sys.argv[0]

> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_timestamp():
>      import datetime
>      return datetime.datetime.strftime(datetime.datetime.now(), '%Y-%m-%d %H:%M:%S')
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> def create_folder(sPath):
>      import os
>      import errno
>      #if not os.path.exists(directory):
>      #    os.makedirs(directory)
>      try:
>          os.makedirs(sPath, exist_ok=True)
>      except OSError as exception:
>          #if exception.errno != errno.EEXIST:
>          print("ERROR #" + str(exception.errno) + "IN makedirs")
>          raise
>
> # //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> def create_folders(sFolderPathTemplate:str="", iFromCount:int=1, iToCount:int=0, iCountWidth:int=0):
>      # MAKE SURE TEMPLATE'S A STRING. OH, IS THIS NOT "PYTHONIC"? WELL IT'S MORE READABLE
>      if is_string(sFolderPathTemplate) == False:
>          iFromCount = 1; iToCount = 0;
More readable?  The pythonic version would be simply to not use this at all.  Now, which is more 
readable -- this crap or blank lines?
>
>      sName = ""
>      sCount = ""
This is not BASIC, you don't need to pre-declare your variables.

>      iLoop = iFromCount
>      while (iLoop <= iToCount) and (len(sFolderPathTemplate) > 0):
Why check for the template?  It's a global variable, you know it's valid.  If not, you deserve 
your errors.

>          sCount = "{0}".format(iLoop)
>          if (iCountWidth > 0):
>              sCount = sCount.zfill(iCountWidth)
>          sName = sFolderPathTemplate.replace("<iCount/>", sCount)
VERY tricky here, but...
            sCount = "{{:0{}}.format(iCountWidth)" if iCountWidth else "{}"
            sName = "{} {}".format(sFolderPathTemplate, sCount).format(iLoop)
More direct...
            if iCountWidth:
                fmt = "{{:0{}}}".format(iCountWidth)
                sCount = fmt.format(iLoop)
            else:
                sCount = str(iLoop)
            sName = "{} {}".format(sFolderPathTemplate, sCount)
Note this assumes the <iCount/> has been deleted from the template string as noted above.

>          create_folder(sName)
>          iLoop = iLoop + 1
More convenient and more pythonic:  iLoop += 1
>
> ############################################################################################################################################################
> # MAIN LOGIC
>
> def main():
>      # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>      # MAKE SURE PYTHON VERSION IS CORRECT
>      exit_if_wrong_python_version(sForPythonVersion)
>      print("PYTHON VERSION (" + get_exact_python_version() + ") MATCHES REQUIRED VERSION (" + sForPythonVersion + ")")
>      #print("get_python_version       returns \"" + get_python_version()       + "\"")
>      #print("get_exact_python_version returns \"" + get_exact_python_version() + "\"")
Perhaps unnecessary to check the version, but YMMV...  However, the 
exit_if_wrong_python_version() aborts with an error message, why bother to state that the 
version is good?  If it isn't the program doesn't run anyway.
>
>      # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>      # PRINT START TIMESTAMP
>      print("" + get_timestamp() + " " + get_script_filename() + " STARTED.")
>
>      # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>      # DO WHAT WE CAME TO DO
>      # IT'S PROBABLY BAD FORM TO REFERENCE GLOBAL VARIABLES INSIDE THE SCOPE OF A FUNCTION?
>      # BUT I WANT TO MAKE IT EASY TO CONFIGURE THE SCRIPT JUST BY CHANGING A COUPLE OF LINES
>      # AT THE TOP, WITHOUT HAVING TO SEARCH THROUGH THE CODE
>      create_folders(
>          sFolderPathTemplate,
>          iFromCount,
>          iToCount,
>          iCountWidth)
As you say, these are all global variables and are therefore already available to the 
create_folders() function, so why bother passing them as parameters?  Your reason for globals 
here is probably valid, especially since they are used as constants rather than variables.  But 
in general (in all programming languages) it is usually better and safer to avoid globals if 
possible.
>
>      # ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>      # NOW FINISH
>      print("" + get_timestamp() + " " + get_script_filename() + " FINISHED.")
>      #import os
>      #os._exit(0)
Yes, your exit() is redundant and you are correct to comment it out.

But again I suggest that you get used to using print formatting, it is really versatile.

-- 
      -=- Larry -=-

[toc] | [prev] | [next] | [standalone]

#109813

First of all welcome :)

The other suggestions you've received so far are good so I won't repeat
them... (note that in particular I've reused the names you've chosen in
your program where I've given code examples, but that's purely for clarity
and I agree with the others who've said you should use a more pythonic
naming convention)

On Fri, 10 Jun 2016, 23:52 mad scientist jr, <mad.scientist.jr@gmail.com>
wrote:

> Is this group appropriate for that kind of thing?
> (If not sorry for posting this here.)
>
> So I wanted to start learning Python, and there is soooo much information
> online, which is a little overwhelming. I really learn best from doing,
> especially if it's something actually useful. I needed to create a bunch of
> empty folders, so I figured it was a good exercise to start learning Python.
> Now that it's done, I am wondering what kind of things I could do better.
> Here is the code:
>
> # WELCOME TO MY FIRST PYTHON SCRIPT!
> # FIRST OF ALL, IT ***WORKS***!!! YAY!
> # I AM A VBA AND JAVASCRIPT PROGRAMMER,
> # SO IT IS PROBABLY NOT VERY "PYTHONIC",
> # SO PLEASE FEEL FREE TO TEAR THE SCRIPT A NEW ONE
> # AFTER YOU ARE SHOCKED BY THIS BAD CODE,
> # ALL I ASK IS THAT IF YOU THINK SOMETHING IS BAD,
> # PLEASE POST AN EXAMPLE OF THE "CORRECT" WAY OF DOING IT
> # COMING FROM VBA, I KNOW A LITTLE OOP,
> # BUT NOT C++ C# JAVA STUFF LIKE "INTERFACES" AND "ABSTRACT BASE CLASSES"
> # SO IF YOU GET INTO SUCH TERMINOLOGY MY EYES MIGHT START TO GLAZE OVER
> # UNLESS YOU CARE TO EXPLAIN THAT TOO
>
Don't worry, Python doesn't have "interfaces" like Java (though it does
have multiple inheritance instead, which is more powerful), and it's not
necessary to learn about advanced things like abstract classes until you're
comfortable :)

>
>
> ############################################################################################################################################################
> # GLOBAL VALUES
> sForPythonVersion="3"
> #sFolderPathTemplate = "c:\\myscripts\\MP3 Disc <iCount/>"
> sFolderPathTemplate = "c:\\temp\\MP3 Disc <iCount/>"
>
It's possible in Python to use forward slashes in paths, even on Windows,
which would allow you to avoid the double slashes above (and can make your
code more easily ported to other OSes, which pretty universally use a '/'
in paths.
I would also suggest you use python's string formatting capabilities
instead here (see below where I use this FOLDER_TEMPLATE string):

FOLDER_TEMPLATE = 'C:/temp/MP3 Disk {count:03}'

iFromCount = 1
> iToCount = 250
> iCountWidth = 3
>
>
> ################################################################################################################################################################
> # SUPPORT FUNCTIONS
>
> #
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def is_string(myVar): # is_string IS MORE READABLE THAN isinstance (PLAIN
> ENGLISH!)
>     #PYTHON 3 IS NOT LIKING THIS: return ( isinstance(myVar, str) or
> isinstance(myVar, unicode) )
>     #PYTHON 3 IS NOT LIKING THIS: return isinstance(myVar, basestr)
>     return isinstance(myVar, str)
>

I know this one's been asked already but I think it deserves expanding on.
In general in Python it's recommended that you don't try too hard (if at
all) to check types of things. Think about what purpose this serves.
Pythonic programs follow what's known as "duck typing". i.e. if it quacks
like a duck then we can treat it like it's a duck.
This lets you write code which expects, for example something like a
string, or a dict or list, but isn't strictly bound to those types. That
allows other programmers to create their own classes, perhaps a subclass of
dict or list, or some unconnected class which just happens to be list-like
enough, and it can just work with your code.

Of course there are places where you do need to check the type of
something, but if you think you need to it's always worth really asking
yourself why you think that.
(I think this is particularly true when it comes to `str`. Most classes can
be represented one way or another as a string, so why not let people pass
you anything if you're going to read it as a string anyway? Refusing to
accept a non-string is more work for you and more work for them for little
benefit)

#
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # THIS IS SOME SAMPLE FUNCTION FROM A BOOK I AM READING "PYTHON IN EASY
> STEPS"
> def strip_one_space(s):
>     if s.endswith(" "): s = s[:-1]
>     if s.startswith(" "): s = s[1:]
>     return s
>
> #
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_exact_python_version():
>     import sys
>

Imports belong at the top of the file unless there's a good reason you
don't always want the module imported (e.g. if it's really large or doesn't
exist on all the systems your program runs on and isn't a strict
requirement). `sys` or `os` definitely don't fall into that category (and I
see you've imported them more than once!)

    sVersion = ".".join(map(str, sys.version_info[:3]))
>     sVersion = sVersion.strip()
>     return sVersion
>
> #
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # TO DO: RETURN TO THE LEFT OF FIRST "."
> def get_python_version():
>     sVersion = get_exact_python_version()
>     return sVersion[0:1]
>

Why not just look up sys.version_info[0] instead of having this function?

#
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> # CHECK PYTHON VERSION, IF IT'S WRONG THEN GRACEFULLY EXIT BEFORE IT BLOWS
> UP
> # (DAMN, PYTHON 2.x STILL COMPLAINS WITH SYNTAX ERRORS!!)
> # MAYBE THIS COULD STILL BE USEFUL FOR CHECKING THE SUB-VERSION, IN THAT
> CASE
> # TO DO: MORE GRANULAR CHECK, EG IF VERSION >= 3.5.0
> def exit_if_wrong_python_version(sRightVersion):
>     import os
>     sCurrentVersion = get_python_version()
>     if (sCurrentVersion != sRightVersion):
>         print("" +
>               "Wrong Python version (" +
>               sCurrentVersion +
>               "), this script should be run using Python " +
>               sRightVersion +
>               ".x. Exiting..."
>               )
>         os._exit(0)


Better would be just let it crash. That way you can see a stacktrace and
work out how to fix it. There's a place for doing version checking like
this, but small single-purpose scripts should be small and easy to follow.
(I have a feeling you should be using sys.exit() rather than os._exit()
too, maybe someone else can confirm)

#
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_script_filename():
>     import os

    return os.path.basename(__file__)
>

This function's name is nearly as long as the code you're abstracting. Is
it worth having a function for this?

#
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
> def get_timestamp():
>     import datetime
>     return datetime.datetime.strftime(datetime.datetime.now(), '%Y-%m-%d
> %H:%M:%S')
>
> #
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> def create_folder(sPath):
>     import os
>     import errno
>     #if not os.path.exists(directory):
>     #    os.makedirs(directory)
>     try:
>         os.makedirs(sPath, exist_ok=True)
>     except OSError as exception:
>         #if exception.errno != errno.EEXIST:
>         print("ERROR #" + str(exception.errno) + "IN makedirs")
>         raise
>

If you're targetting Python 3 only you should be able to choose the more
specific subclasses of OSError such as FileExistsError rather than worrying
about the errno. If you want to catch *some* of OSError's subclasses but
not others that's possible like:

try:
    ...
except (FileExistsError, IsADirectoryError) as e:
    ...
    raise

#
> //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
>
> def create_folders(sFolderPathTemplate:str="", iFromCount:int=1,
> iToCount:int=0, iCountWidth:int=0):
>     # MAKE SURE TEMPLATE'S A STRING. OH, IS THIS NOT "PYTHONIC"? WELL IT'S
> MORE READABLE
>     if is_string(sFolderPathTemplate) == False:
>         iFromCount = 1; iToCount = 0;
>
>     sName = ""
>     sCount = ""
>
There's no need to initialize variables before they are used

>     iLoop = iFromCount
>     while (iLoop <= iToCount) and (len(sFolderPathTemplate) > 0):
>         sCount = "{0}".format(iLoop)
>
str(iLoop) would have done fine

>         if (iCountWidth > 0):
>             sCount = sCount.zfill(iCountWidth)
>         sName = sFolderPathTemplate.replace("<iCount/>", sCount)
>         create_folder(sName)
>         iLoop = iLoop + 1
>
But in Python it's not necessary to manage your own loops like this.

It would be better rewritten as a for loop:

for i in range(1, iToCount + 1):
    create_folder(FOLDER_TEMPLATE.format(count=i))

Note I haven't had to deal with padding the counter as the format spec in
FOLDER_TEMPLATE deals with that, I don't need to manually increment a loop
counter, for and range are working together to give me that.

############################################################################################################################################################
> # MAIN LOGIC
>
> def main():
>     #
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # MAKE SURE PYTHON VERSION IS CORRECT
>     exit_if_wrong_python_version(sForPythonVersion)
>     print("PYTHON VERSION (" + get_exact_python_version() + ") MATCHES
> REQUIRED VERSION (" + sForPythonVersion + ")")
>
Look up string formatting (things like `'Python version: {}. Required
version: {}'.format(get_exact_python_version(), sForPythonVersion)`) as an
alternative to using + multiple times. It makes things more manageable and
is much more powerful.

    #print("get_python_version       returns \"" + get_python_version()
>    + "\"")
>     #print("get_exact_python_version returns \"" +
> get_exact_python_version() + "\"")
>
>     #
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # PRINT START TIMESTAMP
>     print("" + get_timestamp() + " " + get_script_filename() + " STARTED.")
>
>     #
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # DO WHAT WE CAME TO DO
>     # IT'S PROBABLY BAD FORM TO REFERENCE GLOBAL VARIABLES INSIDE THE
> SCOPE OF A FUNCTION?
>     # BUT I WANT TO MAKE IT EASY TO CONFIGURE THE SCRIPT JUST BY CHANGING
> A COUPLE OF LINES
>     # AT THE TOP, WITHOUT HAVING TO SEARCH THROUGH THE CODE
>     create_folders(
>         sFolderPathTemplate,
>         iFromCount,
>         iToCount,
>         iCountWidth)
>
>     #
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>     # NOW FINISH
>     print("" + get_timestamp() + " " + get_script_filename() + "
> FINISHED.")
>

If you were using a *NIX OS I would suggest dropping your start and end
timestamps and just calling your script via the 'time' command, but
apparently Windows doesn't have that :( however I would perhaps move them
out of your `main()` function and wrapping the `main()` call below with
them instead. I don't feel like they are really "part of the program". That
may just be my taste though.

    #import os
>     #os._exit(0)
>
>
> ############################################################################################################################################################
>
> main()
>

This is good, you've not just put your logic in the body of the script, it
could be slightly better though. The standard pattern goes:

if __name__ == '__main__':
    main()

Which means if you were to import your script as a library `main()` won't
be run, allowing you to reuse functions easily, and making writing tests
much easier.

>

[toc] | [prev] | [next] | [standalone]

#109823

Thanks to everyone for your replies.  I see my script was as horrific as I feared, but I read all the responses and made a few changes.  I'm not 100% sold on not checking types, but took it out, because it made sense that other programmers might want to use some custom type with my functions for their own nefarious purposes.  

One question I have is, can someone point me to a full listing of all the error types I can trap for?  It seems like a Pandora's box, trying to think of all the things that could go wrong, which is why I originally just printed the error #. Is it better to not trap errors, and let the stack trace tell what went wrong?  Does it give all the information on the error type etc.?

Anyway, for those charitable (or masochistic, or both) enough to critique my code again, here is the updated version:

# For Python 3.x

# This script creates multiple numbered empty folders
# in the desired location. To change the folder names
# or location, edit function get_default_options.

###############################################################################
# REFERENCE MODULES
###############################################################################
import datetime
import os
import errno
import sys

###############################################################################
# SUPPORT FUNCTIONS
###############################################################################

# returns: dictionary containing options for this script
def get_default_options():
    dict = {
        "s_for_python_version": "3",
        "s_folder_path_template": "C:/temp/test/MP3 Disk {count:03}", 
        "i_from_count": 3,
        "i_to_count": 7,
        }
    return dict

# returns: string containing exact version #, eg "3.5.1"
# TODO: update to use
#   sys.version_info[:3] by itself gives a three-element tuple.
#   Probably easier to use than thestring version. 
#   A couple alternatives: 
#   sys.version[:5] or perhaps a bit safer -- sys.version.split()[0] both directly give you the 
#  string version.  (Note this uses version not version_info.)
def get_exact_python_version():
    s_version = ".".join(map(str, sys.version_info[:3]))
    s_version = s_version.strip()
    return s_version

# returns: string containing general version #, eg "3"
# TODO: return to the left of first "."
def get_general_python_version():
    s_version = get_exact_python_version()
    return s_version[0]

# checks python version
# if it's wrong then gracefully exit before it blows up
# (damn, python 2.x still complains with syntax errors!!)
#
# receives:
#   s_right_version (string) = python version # to check against
# 
# TODO: more granular check, eg if version >= 3.5.0
def exit_if_wrong_python_version(s_right_version):
    s_current_version = get_general_python_version()
    if (s_current_version != s_right_version):
        print(
            "Wrong Python version ({}), "
            "this script should be run using " 
            "Python {}.x,  Exiting..."
            "".format(s_current_version, s_right_version))
        sys.exit() # SAME AS os._exit(0)

# assists in script readability
# returns: string containing name of the current script
def get_script_filename():
    return sys.argv[0]

# returns: string containing the current date/time in the format eg 2016-05-22 13:01:55
def get_timestamp():
    return datetime.datetime.strftime(datetime.datetime.now(), '%Y-%m-%d %H:%M:%S')

# creates a folder at the specified path
# receives:
#   s_path (string) = full path of folder to create
def create_folder(s_path):
    try:
        os.makedirs(s_path, exist_ok=True)
    except (FileExistsError, IsADirectoryError) as e:
        print("FileExistsError IN makedirs")
        raise
        return False
    except OSError as exception:
        print("ERROR #" + str(exception.errno) + "IN makedirs")
        raise
        return False
    print("" + get_timestamp() + " " + "Created folder: " + s_path + "")
    
# creates multiple numbered folders named per template
# receives:
#   s_folder_path_template (string) = template containing full path of folder to create,
#                                     where "{count:n}" is replaced by the folder count (n digits)
#   i_from_count (int) = number to begin counting at
#   i_to_count (int) = number to stop counting after
#
# returns: count of folders created, 0 if error or none
def create_folders(
        s_folder_path_template:str="",
        i_from_count:int=1,
        i_to_count:int=0
        ):
    i_count=0
    for i_loop in range(i_from_count, i_to_count + 1): 
        create_folder(s_folder_path_template.format(count=i_loop))
        i_count += 1
    
    return i_count
    
###############################################################################
# MAIN LOGIC
###############################################################################

def main():
    options_dict = get_default_options()
    exit_if_wrong_python_version(options_dict["s_for_python_version"])
    
    print("+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++")
    print("" + get_timestamp() + " " + get_script_filename() + " started.")
    
    i_total_created = create_folders(
        options_dict["s_folder_path_template"],
        options_dict["i_from_count"],
        options_dict["i_to_count"])

    print("" + get_timestamp() + " " + str(i_total_created) + " folders created.")
    print("" + get_timestamp() + " " + get_script_filename() + " finished.")

###############################################################################
# GLOBAL CODE (minimal!)
###############################################################################
if __name__ == '__main__': 
    main()

[toc] | [prev] | [next] | [standalone]

#109824

For those who don't want to have to wade through comments, here is a version without so many comments:

# For Python 3.x
# This script creates multiple numbered empty folders
# in the desired location. To change the folder names
# or location, edit function get_default_options.

import datetime
import os
import errno
import sys

###############################################################################
# EDIT VALUES HERE TO CUSTOMIZE THE OUTPUT
def get_default_options():
    dict = {
        "s_for_python_version": "3",
        "s_folder_path_template": "C:/temp/test/MP3 Disk {count:03}", 
        "i_from_count": 3,
        "i_to_count": 7,
        }
    return dict
###############################################################################

def get_exact_python_version():
    s_version = ".".join(map(str, sys.version_info[:3]))
    s_version = s_version.strip()
    return s_version

def get_general_python_version():
    s_version = get_exact_python_version()
    return s_version[0]

def exit_if_wrong_python_version(s_right_version):
    s_current_version = get_general_python_version()
    if (s_current_version != s_right_version):
        print(
            "Wrong Python version ({}), "
            "this script should be run using " 
            "Python {}.x,  Exiting..."
            "".format(s_current_version, s_right_version))
        sys.exit()

def get_script_filename():
    return sys.argv[0]

def get_timestamp():
    return datetime.datetime.strftime(datetime.datetime.now(), '%Y-%m-%d %H:%M:%S')

def create_folder(s_path):
    try:
        os.makedirs(s_path, exist_ok=True)
    except (FileExistsError, IsADirectoryError) as e:
        print("FileExistsError IN makedirs")
        raise
        return False
    except OSError as exception:
        print("ERROR #" + str(exception.errno) + "IN makedirs")
        raise
        return False
    print("" + get_timestamp() + " " + "Created folder: " + s_path + "")
    
def create_folders(
        s_folder_path_template:str="",
        i_from_count:int=1,
        i_to_count:int=0
        ):
    i_count=0
    for i_loop in range(i_from_count, i_to_count + 1): 
        create_folder(s_folder_path_template.format(count=i_loop))
        i_count += 1
    
    return i_count
    
def main():
    options_dict = get_default_options()
    exit_if_wrong_python_version(options_dict["s_for_python_version"])
    
    print("+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++")
    print("" + get_timestamp() + " " + get_script_filename() + " started.")
    
    i_total_created = create_folders(
        options_dict["s_folder_path_template"],
        options_dict["i_from_count"],
        options_dict["i_to_count"])

    print("" + get_timestamp() + " " + str(i_total_created) + " folders created.")
    print("" + get_timestamp() + " " + get_script_filename() + " finished.")

if __name__ == '__main__': 
    main()

[toc] | [prev] | [next] | [standalone]

#109825

Look into docstrings. They will make your code much more readable to a
Python reader.
On Sat, Jun 11, 2016 at 2:16 PM mad scientist jr <mad.scientist.jr@gmail.com>
wrote:

> For those who don't want to have to wade through comments, here is a
> version without so many comments:
>
> # For Python 3.x
> # This script creates multiple numbered empty folders
> # in the desired location. To change the folder names
> # or location, edit function get_default_options.
>
> import datetime
> import os
> import errno
> import sys
>
>
> ###############################################################################
> # EDIT VALUES HERE TO CUSTOMIZE THE OUTPUT
> def get_default_options():
>     dict = {
>         "s_for_python_version": "3",
>         "s_folder_path_template": "C:/temp/test/MP3 Disk {count:03}",
>         "i_from_count": 3,
>         "i_to_count": 7,
>         }
>     return dict
>
> ###############################################################################
>
> def get_exact_python_version():
>     s_version = ".".join(map(str, sys.version_info[:3]))
>     s_version = s_version.strip()
>     return s_version
>
> def get_general_python_version():
>     s_version = get_exact_python_version()
>     return s_version[0]
>
> def exit_if_wrong_python_version(s_right_version):
>     s_current_version = get_general_python_version()
>     if (s_current_version != s_right_version):
>         print(
>             "Wrong Python version ({}), "
>             "this script should be run using "
>             "Python {}.x,  Exiting..."
>             "".format(s_current_version, s_right_version))
>         sys.exit()
>
> def get_script_filename():
>     return sys.argv[0]
>
> def get_timestamp():
>     return datetime.datetime.strftime(datetime.datetime.now(), '%Y-%m-%d
> %H:%M:%S')
>
> def create_folder(s_path):
>     try:
>         os.makedirs(s_path, exist_ok=True)
>     except (FileExistsError, IsADirectoryError) as e:
>         print("FileExistsError IN makedirs")
>         raise
>         return False
>     except OSError as exception:
>         print("ERROR #" + str(exception.errno) + "IN makedirs")
>         raise
>         return False
>     print("" + get_timestamp() + " " + "Created folder: " + s_path + "")
>
> def create_folders(
>         s_folder_path_template:str="",
>         i_from_count:int=1,
>         i_to_count:int=0
>         ):
>     i_count=0
>     for i_loop in range(i_from_count, i_to_count + 1):
>         create_folder(s_folder_path_template.format(count=i_loop))
>         i_count += 1
>
>     return i_count
>
> def main():
>     options_dict = get_default_options()
>     exit_if_wrong_python_version(options_dict["s_for_python_version"])
>
>
> print("+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++")
>     print("" + get_timestamp() + " " + get_script_filename() + " started.")
>
>     i_total_created = create_folders(
>         options_dict["s_folder_path_template"],
>         options_dict["i_from_count"],
>         options_dict["i_to_count"])
>
>     print("" + get_timestamp() + " " + str(i_total_created) + " folders
> created.")
>     print("" + get_timestamp() + " " + get_script_filename() + "
> finished.")
>
> if __name__ == '__main__':
>     main()
> --
> https://mail.python.org/mailman/listinfo/python-list
>

[toc] | [prev] | [next] | [standalone]

#109826

On 2016-06-11 18:59, mad scientist jr wrote:
> Thanks to everyone for your replies.  I see my script was as horrific as I feared, but I read all the responses and made a few changes.  I'm not 100% sold on not checking types, but took it out, because it made sense that other programmers might want to use some custom type with my functions for their own nefarious purposes.
>
> One question I have is, can someone point me to a full listing of all the error types I can trap for?  It seems like a Pandora's box, trying to think of all the things that could go wrong, which is why I originally just printed the error #. Is it better to not trap errors, and let the stack trace tell what went wrong?  Does it give all the information on the error type etc.?
>
Catch those exceptions that you can do something about.

If, say, it complains that it can't create a folder, you'll need to 
decide what you should do about it. You might decide that the best way 
to handle it is to report it to the user and then continue, or report it 
to the user and then stop. Do whatever makes most sense.

If it complains about something unexpected, it's probably best to report 
it and then just stop, i.e. let unknown exceptions propagate.

As Python is an extensible language, there'll never be a complete list 
of exceptions; just catch what you're prepared to handle.

> Anyway, for those charitable (or masochistic, or both) enough to critique my code again, here is the updated version:
>
> # For Python 3.x
>
> # This script creates multiple numbered empty folders
> # in the desired location. To change the folder names
> # or location, edit function get_default_options.
>
Drop the next 3 comment lines. They add visual clutter, and no useful info.

> ###############################################################################
> # REFERENCE MODULES
> ###############################################################################
> import datetime
> import os
> import errno
> import sys
>
> ###############################################################################
> # SUPPORT FUNCTIONS
> ###############################################################################
>
> # returns: dictionary containing options for this script
> def get_default_options():
>     dict = {
>         "s_for_python_version": "3",
>         "s_folder_path_template": "C:/temp/test/MP3 Disk {count:03}",
>         "i_from_count": 3,
>         "i_to_count": 7,
>         }
>     return dict
>
> # returns: string containing exact version #, eg "3.5.1"
> # TODO: update to use
> #   sys.version_info[:3] by itself gives a three-element tuple.
> #   Probably easier to use than thestring version.
> #   A couple alternatives:
> #   sys.version[:5] or perhaps a bit safer -- sys.version.split()[0] both directly give you the
> #  string version.  (Note this uses version not version_info.)
> def get_exact_python_version():
>     s_version = ".".join(map(str, sys.version_info[:3]))

The string produced by the preceding line won't start or end with any 
whitespace, so the next line is pointless.

>     s_version = s_version.strip()
>     return s_version
>
> # returns: string containing general version #, eg "3"
> # TODO: return to the left of first "."
> def get_general_python_version():

It's called the "major" version. You're going the long way round! The 
simplest way to get it is directly from sys.version_info.

>     s_version = get_exact_python_version()
>     return s_version[0]
>
> # checks python version
> # if it's wrong then gracefully exit before it blows up
> # (damn, python 2.x still complains with syntax errors!!)
> #
> # receives:
> #   s_right_version (string) = python version # to check against
> #
> # TODO: more granular check, eg if version >= 3.5.0
> def exit_if_wrong_python_version(s_right_version):
>     s_current_version = get_general_python_version()

The conditions of 'if' statements don't need to be wrapped in (...).

>     if (s_current_version != s_right_version):
>         print(
>             "Wrong Python version ({}), "
>             "this script should be run using "
>             "Python {}.x,  Exiting..."
>             "".format(s_current_version, s_right_version))
>         sys.exit() # SAME AS os._exit(0)
>
> # assists in script readability
> # returns: string containing name of the current script
> def get_script_filename():
>     return sys.argv[0]
>
> # returns: string containing the current date/time in the format eg 2016-05-22 13:01:55
> def get_timestamp():
 >     return datetime.datetime.strftime(datetime.datetime.now(), 
'%Y-%m-%d %H:%M:%S')

datetime instances have a .strftime method, so you can shorten that to:

     return datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')

>
> # creates a folder at the specified path
> # receives:
> #   s_path (string) = full path of folder to create
> def create_folder(s_path):
>     try:
>         os.makedirs(s_path, exist_ok=True)
>     except (FileExistsError, IsADirectoryError) as e:

There's little point in catching an exception, printing a message, and 
then re-raising the exception. The exception's traceback already gives 
you much more information that the printed message.

>         print("FileExistsError IN makedirs")
>         raise

You'll never get here because you've just re-raised the exception.

>         return False
>     except OSError as exception:

Similar remarks to above.

>         print("ERROR #" + str(exception.errno) + "IN makedirs")
>         raise
>         return False

The initial ""+ is pointless.

>     print("" + get_timestamp() + " " + "Created folder: " + s_path + "")
>
> # creates multiple numbered folders named per template
> # receives:
> #   s_folder_path_template (string) = template containing full path of folder to create,
> #                                     where "{count:n}" is replaced by the folder count (n digits)
> #   i_from_count (int) = number to begin counting at
> #   i_to_count (int) = number to stop counting after
> #
> # returns: count of folders created, 0 if error or none
> def create_folders(
>         s_folder_path_template:str="",
>         i_from_count:int=1,
>         i_to_count:int=0
>         ):
>     i_count=0
>     for i_loop in range(i_from_count, i_to_count + 1):
>         create_folder(s_folder_path_template.format(count=i_loop))
>         i_count += 1
>
>     return i_count
>
> ###############################################################################
> # MAIN LOGIC
> ###############################################################################
>
> def main():
>     options_dict = get_default_options()
>     exit_if_wrong_python_version(options_dict["s_for_python_version"])
>

There's an easier to make the repeated string: "+" * 79

>     print("+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++")

The initial ""+ is pointless.

>     print("" + get_timestamp() + " " + get_script_filename() + " started.")
>
>     i_total_created = create_folders(
>         options_dict["s_folder_path_template"],
>         options_dict["i_from_count"],
>         options_dict["i_to_count"])
>
>     print("" + get_timestamp() + " " + str(i_total_created) + " folders created.")
>     print("" + get_timestamp() + " " + get_script_filename() + " finished.")
>
> ###############################################################################
> # GLOBAL CODE (minimal!)
> ###############################################################################
> if __name__ == '__main__':
>     main()
>

[toc] | [prev] | [next] | [standalone]

#109850

Thanks for your reply!

On Saturday, June 11, 2016 at 2:41:39 PM UTC-4, MRAB wrote:
> Drop the next 3 comment lines. They add visual clutter, and no useful info.
> > ###############################################################################
> > # REFERENCE MODULES
> > ###############################################################################

I'm not going to argue minor points at length, because others have said the same thing here, but I will push back a little and explain that what you guys consider visual clutter, I find helps me to visually break up and quickly identify sections in my code, which is why I like those separators.

> There's an easier to make the repeated string: "+" * 79
> >     print("+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++")

Thanks, that's a good tip. In this case I might prefer showing the full line of +s because that way, wysiwyg, it's just easier to visualize the output.

Thanks again for the input. . . I will further digest what you all said and study some more (including the docstrings)

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.python

csiph-web