Streamline and standardise message making; misc bug fixes; bump version

Makefile

 
 git:
-	cd /home/g/projects/superhelp && nosetests
+	/home/g/projects/superhelp/superhelp/env/bin/nosetests
 	sed -i 's/^test_misc()/# test_misc()/' /home/g/projects/superhelp/tests/*.py
 	sed -i 's/RECORD_AST = t/RECORD_AST = f/' /home/g/projects/superhelp/superhelp/conf.py
 	sed -i 's/DEV_MODE = t/DEV_MODE = f/' /home/g/projects/superhelp/superhelp/conf.py

README.md

 # https://git.nzoss.org.nz/pyGrant/superhelp
 
-version number: 0.9.1
+version number: 0.9.2
 author: Grant Paton-Simpson
 
 ## Overview

setup.py

@@ -2,7 +2,7 @@ from setuptools import setup, find_packages  # @UnresolvedImport
 from codecs import open
 from os import path
 
-__version__ = '0.9.1'
+__version__ = '0.9.2'
 
 here = path.abspath(path.dirname(__file__))
 

superhelp/advisors/decorator_advisors.py

@@ -28,16 +28,16 @@ def decorator_overview(block_dets, *, repeated_message=False):
             if namespace:
                 name = f"{namespace}.{name}"
         decorator_names.append(name)
+
     dec_name_list = get_nice_str_list(decorator_names, quoter='`')
     plural = 's' if len(decorator_names) > 1 else ''
-    brief_msg = layout(f"""\
+    summary = layout(f"""\
             ### Decorator{plural} used
 
             The code uses the decorator{plural}: {dec_name_list}.
             """)
-    main_msg = brief_msg
     if not repeated_message:
-        main_msg += (
+        dec_dets = (
             layout("""\
 
                 Decorators are a common and handy feature of Python. Using them
@@ -95,8 +95,11 @@ def decorator_overview(block_dets, *, repeated_message=False):
                 say("sausage!")
                 ''', is_code=True)
         )
+    else:
+        dec_dets = ''
+
     message = {
-        conf.BRIEF: brief_msg,
-        conf.MAIN: main_msg,
+        conf.BRIEF: summary,
+        conf.MAIN: summary + dec_dets,
     }
     return message

superhelp/advisors/enumerate_advisors.py

@@ -346,16 +346,15 @@ def manual_incrementing(blocks_dets):
             break
     if not has_incrementing:
         return None
-    brief_msg = layout(f"""\
+
+    summary = layout(f"""\
 
         ### Possible option of using `enumerate()`
 
         It looks like your code is manually incrementing `{incrementing_var}`.
         In Python you can use the `enumerate` function to handle this for you.
         """)
-    main_msg = (
-        brief_msg
-        +
+    demo = (
         layout("""\
 
         Here is an example of the manual approach:
@@ -403,8 +402,9 @@ def manual_incrementing(blocks_dets):
 
         """)
     )
+
     message = {
-        conf.BRIEF: brief_msg,
-        conf.MAIN: main_msg,
+        conf.BRIEF: summary,
+        conf.MAIN: summary + demo,
     }
     return message

superhelp/advisors/exception_advisors.py

@@ -4,11 +4,6 @@ from ..utils import get_nice_str_list, int2nice, layout_comment as layout
 
 UNSPECIFIC_EXCEPTION = 'Exception'
 
-def _get_exception_block_comment(exception_block):
-    handlers = get_nice_str_list(exception_block, quoter='`')
-    comment = f"The following exception handlers were detected: {handlers}"
-    return comment
-
 def get_exception_blocks(blocks_dets):
     """
     There can be multiple try-except statements in a snippet so we have to
@@ -38,17 +33,25 @@ def exception_overview(blocks_dets):
     exception_blocks = get_exception_blocks(blocks_dets)
     if not exception_blocks:
         return None
-    brief_msg = '### Exception handling'
+
+    title = layout("""\
+
+        ### Exception handling
+
+        """)
+    block_comment_bits = []
     for n, exception_block in enumerate(exception_blocks, 1):
         counter = '' if len(exception_blocks) == 1 else f" {int2nice(n)}"
-        brief_msg += layout(f"""\
+        handlers = get_nice_str_list(exception_block, quoter='`')
+        block_comment_bits.append(layout(f"""\
 
             #### `try`-`except` block{counter}
+            The following exception handlers were detected: {handlers}
+            """))
+    block_comments = ''.join(block_comment_bits)
 
-            """)
-        brief_msg += _get_exception_block_comment(exception_block)
     message = {
-        conf.BRIEF: layout(brief_msg),
+        conf.BRIEF: title + block_comments,
     }
     return message
 
@@ -60,26 +63,22 @@ def unspecific_exception(blocks_dets):
     exception_blocks = get_exception_blocks(blocks_dets)
     if not exception_blocks:
         return None
-    brief_msg = ''
     unspecific_block_ns = []
-    has_title = False
-    has_unspecific = False
     for n, exception_block in enumerate(exception_blocks, 1):
         only_unspecific = (len(exception_block) == 1
             and exception_block[0] == UNSPECIFIC_EXCEPTION)
         if not only_unspecific:
             continue
-        has_unspecific = True
         unspecific_block_ns.append(n)
-        if not has_title:
-            brief_msg += layout("""\
+    if not unspecific_block_ns:
+        return None
 
-                #### Un-specific `Exception` only in `try`-`except` block(s)
+    title = layout("""\
+
+        #### Un-specific `Exception` only in `try`-`except` block(s)
+
+        """)
 
-                """)
-        has_title = True
-    if not has_unspecific:
-        return None
     n_unspecific = len(unspecific_block_ns)
     if n_unspecific == 1:
         block_n_specific_text = ' has'
@@ -89,7 +88,7 @@ def unspecific_exception(blocks_dets):
             for unspecific_block_n in unspecific_block_ns]
         blocks_ns = get_nice_str_list(unspecific_nice_block_ns, quoter='')
         block_n_specific_text = f"s {blocks_ns} have"
-    brief_msg += layout(f"""\
+    unspecific_warning = layout(f"""\
 
         `try`-`except` block{block_n_specific_text} an un-specific Exception
         only.
@@ -97,15 +96,14 @@ def unspecific_exception(blocks_dets):
         Using the un-specific exception type `Exception` is often completely
         appropriate. But if you are looking for specific exceptions you
         should handle those separately.
-
-        For example:
         """)
-    message = {
-        conf.BRIEF: layout(brief_msg),
-        conf.MAIN: (
-            layout(brief_msg)
-            +
-            layout("""\
+    unspecific_demo = (
+        layout(f"""\
+
+            For example:
+            """)
+        +
+        layout("""\
 
                 try:
                     spec_dicts[idx][spec_type]
@@ -117,6 +115,10 @@ def unspecific_exception(blocks_dets):
                 except Exception as e:
                     print(f"Unexpected exception - details: {e}")
                 """, is_code=True)
-        ),
+        )
+
+    message = {
+        conf.BRIEF: title + unspecific_warning,
+        conf.MAIN: title + unspecific_warning + unspecific_demo,
     }
     return message

superhelp/advisors/for_advisors.py

@@ -40,24 +40,26 @@ def comprehension_option(block_dets, *, repeated_message=False):
         comp_comment = shared.SET_COMPREHENSION_COMMENT
     else:
         return None
-    brief_msg = layout(f"""\
+
+    title = layout(f"""\
+
         ### Possible option of using a {comp_type}
 
         """)
     if not repeated_message:
-        brief_msg += layout(f"""\
+        option = layout(f"""\
             Simple for loops can sometimes be replaced with comprehensions. In
             this case a simple reading of the code suggests a {comp_type} might
             be possible. Of course, only use a comprehension if it makes your
             code easier to understand.
             """)
+    else:
+        option = ''
+
     message = {
-        conf.BRIEF: brief_msg,
-        conf.MAIN: (
-            brief_msg
-            +
-            shared.GENERAL_COMPREHENSION_COMMENT + '\n\n' + comp_comment
-        ),
+        conf.BRIEF: title + option,
+        conf.MAIN: (title + option + shared.GENERAL_COMPREHENSION_COMMENT
+            + '\n\n' + comp_comment),
     }
     return message
 
@@ -140,7 +142,8 @@ def for_index_iteration(block_dets, *, repeated_message=False):
             break
     if not any_incremental_iteration:
         return None
-    brief_msg = layout(f"""\
+
+    summary = layout(f"""\
 
         ### Possible option of using direct iteration
 
@@ -148,7 +151,7 @@ def for_index_iteration(block_dets, *, repeated_message=False):
         indexes. In Python you can iterate directly which is much easier.
         """)
     if not repeated_message:
-        brief_msg += (
+        examples = (
             layout(f"""\
 
                 For example, instead of:
@@ -181,8 +184,11 @@ def for_index_iteration(block_dets, *, repeated_message=False):
 
                 """)
         )
+    else:
+        examples = ''
+
     message = {
-        conf.BRIEF: brief_msg,
+        conf.BRIEF: summary + examples,
     }
     return message
 
@@ -201,7 +207,8 @@ def nested_fors(block_dets, *, repeated_message=False):
             break
     if not nested_iteration:
         return None
-    brief_msg = layout("""\
+
+    summary = layout("""\
 
         ### Possible option of simplifying nested iteration
 
@@ -209,7 +216,7 @@ def nested_fors(block_dets, *, repeated_message=False):
 
         """)
     if not repeated_message:
-        brief_msg += (
+        demo = (
             layout("""\
                 For example, you could replace:
 
@@ -234,9 +241,7 @@ def nested_fors(block_dets, *, repeated_message=False):
                     print(f"{person} might like a {pet} in {year}")
                 """, is_code=True)
         )
-    main_msg = brief_msg
-    if not repeated_message:
-        main_msg += layout("""\
+        pros = layout("""\
 
             Whether this is a good idea or not depends on your specific code but
             using `product` has the advantage of reducing indentation. It also
@@ -244,8 +249,12 @@ def nested_fors(block_dets, *, repeated_message=False):
             every option in the cartesian product of the different collections
             of items.
             """)
+    else:
+        demo = ''
+        pros = ''
+
     message = {
-        conf.BRIEF: brief_msg,
-        conf.MAIN: main_msg,
+        conf.BRIEF: summary + demo,
+        conf.MAIN: summary + demo + pros,
     }
     return message

superhelp/advisors/list_advisors.py

@@ -30,7 +30,7 @@ def get_type_for_example(list_items):
             type4example = DEFAULT_EXAMPLE_TYPE
     return type4example
 
-def _get_additional_main_msg(first_name, first_list_items):
+def _get_detailed_list_comment(first_name, first_list_items):
     """
     :param list first_list_items: note - may be None if a list is defined in a
      function
@@ -54,7 +54,7 @@ def _get_additional_main_msg(first_name, first_list_items):
     friends = ['Selma', 'Willy', 'Principal Skinner']
     family = ['Bart', 'Lisa', 'Marge', 'Homer']
     guests = friends + family
-    additional_main_msg = (
+    detailed_list_comment = (
         layout("""\
 
             Lists, along with dictionaries, are the workhorses of Python data
@@ -145,7 +145,7 @@ def _get_additional_main_msg(first_name, first_list_items):
 
             """)
         )
-    return additional_main_msg
+    return detailed_list_comment
 
 ## only interested in lists when being assigned as a value
 ## (i.e. <body><Assign><value><List> so we're looking for List under value only)
@@ -155,11 +155,15 @@ def list_overview(block_dets, *, repeated_message=False):
     General overview of list taking content details into account.
     """
     list_els = block_dets.element.xpath(ASSIGN_LIST_XPATH)
-    brief_msg = ''
-    main_msg = ''
     plural = 's' if len(list_els) > 1 else ''
     first_name = None
     first_list_items = None
+
+    title = layout(f"""\
+
+        ### List{plural} defined
+
+        """)
     for i, list_el in enumerate(list_els):
         first = (i == 0)
         name = get_assign_name(list_el)
@@ -168,13 +172,6 @@ def list_overview(block_dets, *, repeated_message=False):
         if first:
             first_name = name
             first_list_items = items
-            title = layout(f"""\
-
-                ### List{plural} defined
-    
-                """)
-            brief_msg += title
-            main_msg += title
         if items is None:
             list_desc = layout(f"""\
 
@@ -190,10 +187,8 @@ def list_overview(block_dets, *, repeated_message=False):
     
                 `{name}` is a list with {utils.int2nice(len(items))} items.
                 """)
-        brief_msg += list_desc
-        main_msg += list_desc
     if not repeated_message:
-        brief_msg += layout("""\
+        brief_overview = layout("""\
 
             Lists, along with dictionaries, are the workhorses of Python data
             structures.
@@ -201,11 +196,15 @@ def list_overview(block_dets, *, repeated_message=False):
             Lists have an order, and can contain duplicate items and items of
             different types (usually not advisable).
             """)
-        main_msg += _get_additional_main_msg(
+        detailed_list_comment = _get_detailed_list_comment(
             first_name, first_list_items)
+    else:
+        brief_overview = ''
+        detailed_list_comment = ''
+
     message = {
-        conf.BRIEF: brief_msg,
-        conf.MAIN: main_msg,
+        conf.BRIEF: title + list_desc + brief_overview,
+        conf.MAIN: title + list_desc + detailed_list_comment,
     }
     return message
 
@@ -215,11 +214,9 @@ def mixed_list_types(block_dets, *, repeated_message=False):  # @UnusedVariable
     Warns about lists containing a mix of data types.
     """
     list_els = block_dets.element.xpath(ASSIGN_LIST_XPATH)
-    brief_msg = ''
-    main_msg = ''
+    list_dets = []
     has_mixed = False
-    for i, list_el in enumerate(list_els):
-        first = (i == 0)
+    for list_el in list_els:
         name = get_assign_name(list_el)
         items = code_execution.get_val(  ## might be None if defined in a function
             block_dets.pre_block_code_str, block_dets.block_code_str, name)
@@ -230,27 +227,29 @@ def mixed_list_types(block_dets, *, repeated_message=False):  # @UnusedVariable
             ## No explanation needed if there aren't multiple types.
             continue
         has_mixed = True
-        if first:
-            title = layout(f"""\
-                ### List(s) with mix of different data types
-    
-                """)
-            brief_msg += title
-            main_msg += title
-        mixed_warning = layout(f"""
+        list_dets.append((name, item_type_nice_names))
+    if not has_mixed:
+        return None
+
+    title = layout("""\
+
+        ### List(s) with mix of different data types
+
+        """)
+    mixed_warning_bits = []
+    for name, item_type_nice_names in list_dets:
+        mixed_warning_bits.append(layout(f"""
 
             `{name}` contains more than one data type - which is probably a bad
             idea.
-            """)
-        brief_msg += mixed_warning
-        main_msg += mixed_warning
-        main_msg += layout(f"""\
-             The data types found were: {", ".join(item_type_nice_names)}.
-            """)
-    if not has_mixed:
-        return None
+            """))
+    mixed_warning = ''.join(mixed_warning_bits)
+    mixed_dets = layout(f"""\
+        The data types found were: {", ".join(item_type_nice_names)}.
+        """)
+
     message = {
-        conf.BRIEF: brief_msg,
-        conf.MAIN: main_msg,
+        conf.BRIEF: title + mixed_warning,
+        conf.MAIN: title + mixed_warning + mixed_dets,
     }
     return message

superhelp/advisors/listcomp_advisors.py

@@ -12,29 +12,31 @@ def listcomp_overview(block_dets, *, repeated_message=False):
     comprehension available in Python.
     """
     listcomp_els = block_dets.element.xpath(ASSIGN_LISTCOMP_XPATH)
-    brief_msg = ''
-    plural = 's' if len(listcomp_els) > 1 else ''
-    for i, dict_el in enumerate(listcomp_els):
-        first = (i == 0)
-        name = get_assign_name(dict_el)
+    listcomp_dets = []
+    for listcomp_el in listcomp_els:
+        name = get_assign_name(listcomp_el)
         items = code_execution.get_val(
             block_dets.pre_block_code_str, block_dets.block_code_str, name)
-        if first:
-            title = layout(f"""\
+        listcomp_dets.append((name, items))
+    if not listcomp_dets:
+        return None
 
-                ### List comprehension{plural} used
+    plural = 's' if len(listcomp_dets) > 1 else ''
+    title = layout(f"""\
 
-                """)
-            brief_msg += title
-        brief_msg += layout(f"""
-
-            `{name}` is a list comprehension returning a list
-            with {utils.int2nice(len(items))} items: {items}
-            """)
-    if repeated_message:
-        extra_msg = ''
-    else:
-        extra_msg = (
+        ### List comprehension{plural} used
+
+        """)
+    summary_bits = []
+    for name, items in listcomp_dets:
+        summary_bits.append(layout(f"""
+
+        `{name}` is a list comprehension returning a list
+        with {utils.int2nice(len(items))} items: {items}
+        """))
+    summary = ''.join(summary_bits)
+    if not repeated_message:
+        other_comprehensions = (
             layout(f"""\
                 ### Other "comprehensions"
 
@@ -42,8 +44,9 @@ def listcomp_overview(block_dets, *, repeated_message=False):
             + shared.GENERAL_COMPREHENSION_COMMENT
             + layout("""\
 
-                List comprehensions aren't the only type of comprehension you can
-                make. Python also lets you write Dictionary and Set Comprehensions:
+                List comprehensions aren't the only type of comprehension you
+                can make. Python also lets you write Dictionary and Set
+                Comprehensions:
 
                 """)
             + shared.DICT_COMPREHENSION_COMMENT
@@ -51,12 +54,16 @@ def listcomp_overview(block_dets, *, repeated_message=False):
             + shared.SET_COMPREHENSION_COMMENT
             + '\n\n'
             + layout("""\
-                Pro tip: don't make comprehension *in*comprehensions ;-). If it is
-                hard to read it is probably better written as a looping structure.
+                Pro tip: don't make comprehension *in*comprehensions ;-). If it
+                is hard to read it is probably better written as a looping
+                structure.
                 """)
         )
+    else:
+        other_comprehensions = ''
+
     message = {
-        conf.BRIEF: brief_msg,
-        conf.EXTRA: extra_msg,
+        conf.BRIEF: title + summary,
+        conf.EXTRA: other_comprehensions,
     }
     return message

superhelp/advisors/name_advisors.py

@@ -159,30 +159,39 @@ def unpythonic_name_check(block_dets, *, repeated_message=False):
             dubious_names.append(name)
     if not (reserved_names or bad_names or dubious_names):
         return None
-    title = _get_shamed_names_title(reserved_names, bad_names, dubious_names)
-    brief_msg = layout(f"""\
-        ### {title}
+
+    shamed_names_title = _get_shamed_names_title(
+        reserved_names, bad_names, dubious_names)
+    title = layout(f"""\
+
+        ### {shamed_names_title}
 
         """)
     if reserved_names:
         reserved_names_listed = utils.get_nice_str_list(
             reserved_names, quoter='`')
-        brief_msg += layout(f"""\
+        reserved_comment = layout(f"""\
             Reserved name(s): {reserved_names_listed}
             """)
+    else:
+        reserved_comment = ''
     if bad_names:
         bad_names_listed = utils.get_nice_str_list(bad_names, quoter='`')
-        brief_msg += layout(f"""\
+        bad_comment = layout(f"""\
             Un-pythonic name(s): {bad_names_listed}
             """)
+    else:
+        bad_comment = ''
     if dubious_names:
         dubious_names_listed = utils.get_nice_str_list(
             dubious_names, quoter='`')
-        brief_msg += layout(f"""\
+        dubious_comment = layout(f"""\
             Possibly un-pythonic name(s): {dubious_names_listed}
             """)
+    else:
+        dubious_comment = ''
     if not repeated_message:
-        brief_msg += layout("""\
+        snake_case = layout("""\
 
             Python variables should not named using reserved words e.g.
             `collections` or `sorted`.
@@ -191,9 +200,7 @@ def unpythonic_name_check(block_dets, *, repeated_message=False):
             lower case, with multiple words joined by underscores e.g.
             `high_scores` (not `highScores` or `HighScores`)
             """)
-    main_msg = brief_msg
-    if not repeated_message:
-        main_msg += layout("""\
+        pascal = layout("""\
             In Python class names and named tuples are expected to be in Pascal
             Case (also known as upper camel case) rather than the usual snake
             case. E.g. `collections.ChainMap`
@@ -202,9 +209,15 @@ def unpythonic_name_check(block_dets, *, repeated_message=False):
             with other code e.g. a library the Python is ported from, or the
             non-Python code that Python is wrapping.
             """)
+    else:
+        snake_case = ''
+        pascal = ''
+
     message = {
-        conf.BRIEF: brief_msg,
-        conf.MAIN: main_msg,
+        conf.BRIEF: (title + reserved_comment + bad_comment
+            + dubious_comment + snake_case + pascal),
+        conf.MAIN: (title + reserved_comment + bad_comment
+            + dubious_comment + snake_case + pascal),
     }
     return message
 
@@ -220,29 +233,36 @@ def short_name_check(block_dets, *, repeated_message=False):
             short_names[len(name)].add(name)
     if not short_names:
         return None
-    short_comment = ''
+
+    title = layout("""
+
+        ### Short variable name
+
+        """)
+    short_comment_bits = []
     for length, names in sorted(short_names.items()):
         freq = len(names)
         multiple = (freq > 1)
+        plural = 's' if length > 1 else ''
         if multiple:
             nice_list = get_nice_str_list(list(names), quoter='`')
-            short_comment += (
-                f" {int2nice(freq).title()} variables have names "
-                f"{int2nice(length)} characters long: {nice_list}.")
+            short_comment_bits.append((
+                f"{int2nice(freq)} variables have names "
+                f"{int2nice(length)} character{plural} long: {nice_list}."))
         else:
             name = names.pop()
-            short_comment += f"`{name}` is short."
-    brief_msg = layout(f"""\
-        ### Short variable names
+            short_comment_bits.append(f"`{name}` is short (only "
+                f"{int2nice(length)} character{plural} long).")
+    short_comment = '; '.join(short_comment_bits)
+    sometimes_ok = layout(f"""\
 
         Sometimes, short variable names are appropriate - even conventional -
         but they should be avoided outside of a few special cases. In your code:
 
         {short_comment}
         """)
-    main_msg = brief_msg
     if not repeated_message:
-        main_msg += (
+        idiomatic = (
             layout("""\