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

[MRAB]

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()
>