quality: Improving coding style

hardware/areca.py

@@ -29,9 +29,9 @@ def split_units(lis):
     'If the value have unit, remove it from the value and return the unit name'
     for unit in ['RPM', '%', ' V', ' C', 'Seconds', 'Times', 'MHz', 'KB', 'MB',
                  'GB']:
-        m = re.search("(.*)%s$" % unit, lis[1])
-        if m:
-            lis[1] = m.group(1).replace(' ', '')
+        match = re.search("(.*)%s$" % unit, lis[1])
+        if match:
+            lis[1] = match.group(1).replace(' ', '')
             return unit.replace(' ', '')
     return None
 
@@ -49,9 +49,9 @@ def parse_output(output, rev=False):
         if len(lis) == 2:
             if "GuiErrMsg" in lis[0]:
                 continue
-            m = re.search('\[(Enclosure#.*)', lis[0])
-            if m:
-                append = m.group(1).replace('#', '') + "/"
+            match = re.search('\[(Enclosure#.*)', lis[0])
+            if match:
+                append = match.group(1).replace('#', '') + "/"
                 continue
             if append:
                 lis[0] = "%s%s" % (append, lis[0])

hardware/benchmark/disk.py

@@ -35,8 +35,7 @@
 
 def is_booted_storage_device(disk):
     """Check if a given disk is booted."""
-    cmdline = "grep -w /ahcexport /proc/mounts | cut -d ' ' -f 1 "
-    "| sed -e 's/[0-9]*//g'"
+    cmdline = "grep -w /ahcexport /proc/mounts | cut -d ' ' -f 1 | sed -e 's/[0-9]*//g'"
     if '/dev/' not in disk:
         disk = '/dev/%s' % disk
     grep_cmd = subprocess.Popen(cmdline,

hardware/benchmark/mem.py

@@ -27,10 +27,11 @@
 
 def get_available_memory():
     """Return the total amount of available memory, in bytes."""
-    with open('/proc/meminfo', 'r') as f:
-        for line in f:
+    with open('/proc/meminfo', 'r') as meminfo:
+        for line in meminfo:
             if line.startswith('MemFree:'):
                 return int(line.split()[1]) * 1024
+    return -1
 
 
 def check_mem_size(block_size, cpu_count):

hardware/bios_hp.py

@@ -27,8 +27,8 @@
 from hardware.detect_utils import cmd
 
 
-def get_hp_conrep(hw_lst):
-    for i in hw_lst:
+def get_hp_conrep(hwlst):
+    for i in hwlst:
         if i[0:3] == ('system', 'product', 'vendor'):
             if i[3] not in ['HPE', 'HP']:
                 return True, ""
@@ -42,23 +42,23 @@ def get_hp_conrep(hw_lst):
     return True, return_value
 
 
-def dump_hp_bios(hw_lst):
+def dump_hp_bios(hwlst):
     # handle output injection for testing purpose
-    valid, hp_config = get_hp_conrep(hw_lst)
+    valid, hpconfig = get_hp_conrep(hwlst)
     if not valid:
         return False
 
-    if hp_config:
-        xml = ET.fromstring(hp_config)
+    if hpconfig:
+        xml = ET.fromstring(hpconfig)
         root = xml.iter("Section")
 
         for child in root:
-            hw_lst.append(('hp', 'bios', child.attrib['name'], child.text))
+            hwlst.append(('hp', 'bios', child.attrib['name'], child.text))
 
     return True
 
 
 if __name__ == "__main__":
-    hw_lst = []
-    dump_hp_bios(hw_lst)
-    pprint.pprint(hw_lst)
+    hwlst = []
+    dump_hp_bios(hwlst)
+    pprint.pprint(hwlst)

hardware/cardiff/cardiff.py

@@ -164,16 +164,16 @@ def analyze_data(global_params, pattern, ignore_list, detail, rampup_value=0,
         sys.exit(2)
 
     health_data_file = utils.find_file(path, pattern)
-    if len(health_data_file) == 0:
+    if not health_data_file:
         print("No log file found with pattern %s!" % pattern)
         sys.exit(1)
+
+    if rampup_value == 0:
+        print("### %d files Selected with pattern '%s' ###" %
+              (len(health_data_file), pattern))
     else:
-        if rampup_value == 0:
-            print("### %d files Selected with pattern '%s' ###" %
-                  (len(health_data_file), pattern))
-        else:
-            print("########## Rampup: %d / %d hosts #########" %
-                  (rampup_value, max_rampup_value))
+        print("########## Rampup: %d / %d hosts #########" %
+              (rampup_value, max_rampup_value))
 
     # Extract data from the hw files
     bench_values = []
@@ -245,12 +245,14 @@ def do_plot(current_dir, gpm_dir, main_title, subtitle, name, unit, titles,
             titles_order, expected_value=""):
     filename = current_dir + "/" + name + ".gnuplot"
     process = subprocess.Popen(["gnuplot", "-V"], stdout=subprocess.PIPE)
-    out, err = process.communicate()
+    out, _ = process.communicate()
     version = int(out.split()[1].split('.')[0])
     if version >= 5:
-        gnuplot_arg = lambda x: "ARG" + str(x + 1)
+        def gnuplot_arg(argument):
+            return "ARG" + str(argument + 1)
     else:
-        gnuplot_arg = lambda x: "'$" + str(x) + "'"
+        def gnuplot_arg(argument):
+            return "'$" + str(argument) + "'"
     with open(filename, "a") as ofile:
         shutil.copyfile("%s/graph2D.gpm" % gpm_dir,
                         "%s/graph2D.gpm" % current_dir)
@@ -326,8 +328,8 @@ def extract_hw_info(hardware, level1, level2, level3):
     for entry in hardware:
         if level2 == '*':
             temp_level2 = entry[1]
-        if (level1 == entry[0] and temp_level2 == entry[1] and
-                level3 == entry[2]):
+        if (level1 == entry[0] and temp_level2 == entry[1]
+                and level3 == entry[2]):
             result.append(entry[3])
     return result
 
@@ -465,10 +467,9 @@ def main():
     detail = {'category': '', 'group': '', 'item': ''}
     global_params = {}
     try:
-        opts, args = getopt.getopt(sys.argv[1:], "hp:l:g:c:i:I:r:o:",
-                                   ['pattern', 'log-level', 'group',
-                                    'category', 'item', "ignore",
-                                    "rampup", "output_dir"])
+        opts, _ = getopt.getopt(sys.argv[1:], "hp:l:g:c:i:I:r:o:",
+                                ['pattern', 'log-level', 'group', 'category',
+                                 'item', "ignore", "rampup", "output_dir"])
     except getopt.GetoptError:
         print("Error: One of the options passed "
               "to the cmdline was not supported")
@@ -526,8 +527,7 @@ def main():
             global_params["output_dir"] = arg
 
     if (utils.print_level & utils.Levels.DETAIL) == utils.Levels.DETAIL:
-        if (len(detail['group']) == 0 or len(detail['category']) == 0 or
-                len(detail['item']) == 0):
+        if not detail['group'] or not detail['category'] or not detail['item']:
             print("Error: The DETAIL output requires group, category & item "
                   "options to be set")
             sys.exit(2)
@@ -557,9 +557,9 @@ def main():
             try:
                 if os.path.exists(current_dir):
                     shutil.rmtree(current_dir)
-            except IOError as e:
+            except IOError as myexception:
                 print("Unable to delete directory %s" % current_dir)
-                print(e)
+                print(myexception)
                 sys.exit(2)
 
             temp_rampup_values = [int(name) for name in os.listdir(rampup_dir)
@@ -603,8 +603,8 @@ def main():
 
                 for rampup_value in sorted(rampup_values):
                     metrics = {}
-                    metrics_file = (rampup_dir +
-                                    "/%d/%s/metrics" % (rampup_value, job))
+                    metrics_file = (rampup_dir
+                                    + "/%d/%s/metrics" % (rampup_value, job))
                     if not os.path.isfile(metrics_file):
                         print("Missing metric file for rampup=%d (%s)" %
                               (rampup_value, metrics_file))

hardware/cardiff/check.py

@@ -34,13 +34,13 @@ def search_item(system_list, unique_id, item, regexp, exclude_list=[],
         sets[system[unique_id]] = set()
         current_set = sets[system[unique_id]]
         for stuff in system[item]:
-            m = re.match(regexp, stuff[1])
-            if m:
+            match = re.match(regexp, stuff[1])
+            if match:
                 shall_be_added = False
 
                 # If we have an include_list, only those shall be used
                 # So everything is exclude by default
-                if len(include_list) > 0:
+                if include_list:
                     shall_be_excluded = True
                 else:
                     shall_be_excluded = False
@@ -108,19 +108,19 @@ def compute_deviance_percentage(item, df):
 
 
 def print_detail(detail_options, details, df, matched_category):
-    if (utils.print_level & utils.Levels.DETAIL) != utils.Levels.DETAIL:
+    if (utils.PRINTLEVEL & utils.Levels.DETAIL) != utils.Levels.DETAIL:
         return
-    if len(df.loc[details]) > 0:
+    if df.loc[details]:
         print()
         print("%-34s: %-8s: %s" % (matched_category[0],
-                                   utils.Levels.message[utils.print_level],
+                                   utils.Levels.message[utils.PRINTLEVEL],
                                    detail_options['item']))
         print(df.loc[details])
 
 
 def prepare_detail(detail_options, group_number, category, item, details,
                    matched_category_to_save):
-    if (utils.print_level & utils.Levels.DETAIL) != utils.Levels.DETAIL:
+    if (utils.PRINTLEVEL & utils.Levels.DETAIL) != utils.Levels.DETAIL:
         return
 
     matched_group = ''
@@ -144,12 +144,12 @@ def prepare_detail(detail_options, group_number, category, item, details,
     elif re.search(detail_options['item'], item) is not None:
         matched_item = re.search(detail_options['item'], item).group()
 
-    if (len(matched_group) > 0 and len(matched_category) > 0 and
-            len(matched_item) > 0):
+    if matched_group and matched_category and matched_item:
         details.append(matched_item)
         if matched_category not in matched_category_to_save:
             matched_category_to_save.append(matched_category)
         return matched_category
+
     return ""
 
 
@@ -220,7 +220,7 @@ def logical_disks_perf(system_list, unique_id, group_number, detail_options,
             if perf[2] not in modes and perf_unit in perf[2]:
                 modes.append(perf[2])
 
-    if len(modes) == 0:
+    if modes:
         return
 
     for mode in sorted(modes):
@@ -415,7 +415,7 @@ def print_perf(tolerance_min, tolerance_max, item, df, mode, title,
 
 
 def print_summary(mode, array, array_name, unit, df, item_value=None):
-    if (utils.print_level & utils.Levels.SUMMARY) and (len(array) > 0):
+    if (utils.PRINTLEVEL & utils.Levels.SUMMARY) and array:
         result = []
         before = ""
         after = ""
@@ -437,7 +437,7 @@ def print_summary(mode, array, array_name, unit, df, item_value=None):
         perf_status = ""
         if array_name == "consistent":
             if item_value is not None:
-                if mode == "loops_per_sec" or mode == "bogomips":
+                if mode in ("loops_per_sec", "bogomips"):
                     min_cpu_perf = perf_cpu_tables.get_cpu_min_perf(mode,
                                                                     item_value)
                     if min_cpu_perf == 0:
@@ -447,8 +447,7 @@ def print_summary(mode, array, array_name, unit, df, item_value=None):
                         perf_status = ": " + GREEN + "PERF OK" + WHITE
                     else:
                         perf_status = (": " + RED + "PERF FAIL" + WHITE +
-                                       " as min perf should have been : " +
-                                       str(min_cpu_perf))
+                                       " as min perf should have been : " + str(min_cpu_perf))
         utils.do_print(
             mode, utils.Levels.SUMMARY,
             "%3d %s%-10s%s hosts with %8.2f %-4s as average value and %8.2f "

hardware/cardiff/compare_sets.py

@@ -111,16 +111,16 @@ def compute_similar_hosts_list(systems_groups, new_groups):
     for group in new_groups:
         for systems_group in systems_groups:
             intersection = set.intersection(systems_group, group)
-            if (len(intersection) < len(systems_group) and
-                    len(intersection) > 0):
-                # print("%d vs %d" % (len(intersection), len(systems_group)))
-                # We do have a partial match meaning we shall break
-                # the existing group in pieces
-                difference = set.difference(systems_group, group)
-                # The group we worked on doesn't exist anymore
-                # So let's delete it
-                systems_groups.remove(systems_group)
-
-                # Let's add the two sub groups generated by this split
-                systems_groups.append(intersection)
-                systems_groups.append(difference)
+            if intersection:
+                if len(intersection) < len(systems_group):
+                    # print("%d vs %d" % (len(intersection), len(systems_group)))
+                    # We do have a partial match meaning we shall break
+                    # the existing group in pieces
+                    difference = set.difference(systems_group, group)
+                    # The group we worked on doesn't exist anymore
+                    # So let's delete it
+                    systems_groups.remove(systems_group)
+
+                    # Let's add the two sub groups generated by this split
+                    systems_groups.append(intersection)
+                    systems_groups.append(difference)

hardware/cardiff/perf_cpu_tables.py

@@ -20,13 +20,13 @@ def get_generic_cpu_perf(cpu_struct, cpu_type):
     for cpu_list in sorted(cpu_struct, reverse=True):
         if cpu_list in cpu_type:
             return cpu_struct[cpu_list]
-    else:
-        # Unless, retry by shortening the string by one word
-        if len(cpu_type.split()) > 1:
-            shorten_cpu_type = cpu_type.rsplit(' ', 1)[0]
-            return get_generic_cpu_perf(cpu_struct, shorten_cpu_type)
-        else:
-            return 0
+
+    # Unless, retry by shortening the string by one word
+    if len(cpu_type.split()) > 1:
+        shorten_cpu_type = cpu_type.rsplit(' ', 1)[0]
+        return get_generic_cpu_perf(cpu_struct, shorten_cpu_type)
+
+    return 0
 
 
 def get_loops_per_sec_cpu_min_perf(cpu_type):
@@ -56,7 +56,8 @@ def get_bogomips_cpu_min_perf(cpu_type):
 def get_cpu_min_perf(test_type, cpu_type):
     if "loops_per_sec" in test_type:
         return get_loops_per_sec_cpu_min_perf(cpu_type)
-    elif "bogomips" in test_type:
+
+    if "bogomips" in test_type:
         return get_bogomips_cpu_min_perf(cpu_type)
-    else:
-        return 0
+
+    return 0

hardware/cardiff/utils.py

@@ -31,7 +31,7 @@ class Levels:
 
 
 # Default level is to print everything
-print_level = Levels.INFO | Levels.WARNING | Levels.ERROR
+PRINTLEVEL = Levels.INFO | Levels.WARNING | Levels.ERROR
 
 
 def write_gnuplot_file(filename, index, value):
@@ -41,8 +41,8 @@ def write_gnuplot_file(filename, index, value):
                 myfile.write("%d %.2f\n" % (index, value))
     else:
         new_lines = []
-        with open(filename, "r") as f:
-            lines = (line.rstrip() for line in f)
+        with open(filename, "r") as gnuplotfile:
+            lines = (line.rstrip() for line in gnuplotfile)
             found = False
             for line in lines:
                 if int(line.split()[0].strip()) == index:
@@ -52,13 +52,13 @@ def write_gnuplot_file(filename, index, value):
                     new_lines.append("%s" % (line.strip()))
             if found is False:
                 new_lines.append("%d %.2f" % (index, value))
-        with open(filename, "w") as f:
-            f.write('\n'.join(new_lines) + '\n')
+        with open(filename, "w") as gnuplotfile:
+            gnuplotfile.write('\n'.join(new_lines) + '\n')
 
 
 def do_print(mode, level, string, *args):
-    global print_level
-    if level & int(print_level) != level:
+    global PRINTLEVEL
+    if level & int(PRINTLEVEL) != level:
         return
     final_string = "%-34s: %-8s: " + string
     final_args = (mode, Levels.message[int(level)])
@@ -112,7 +112,7 @@ def find_sub_element(bench_values, unique_id, element, hosts=set()):
             if element in line[0]:
                 stuff.append(line)
 
-        if (len(hosts) == 0) or system[unique_id] in hosts:
+        if not hosts or system[unique_id] in hosts:
             system[element] = stuff
             systems.append(system)