Merge pull request #5043 from Flamefire/comprehensions

boegel · web-flow · commit 76e684d23433 · 2025-11-19T15:56:52.000+01:00
Enable flake8-comprehension code style check and fix issues
diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml
@@ -29,7 +29,7 @@ jobs:
     - name: install Python packages
       run: |
         pip install --upgrade pip
-        pip install --upgrade flake8
+        pip install --upgrade flake8 flake8-comprehensions
 
     - name: Run flake8 to verify PEP8-compliance of Python code
       run: |
diff --git a/easybuild/base/fancylogger.py b/easybuild/base/fancylogger.py
@@ -435,7 +435,7 @@ def thread_name():
     """
     returns the current threads name
     """
-    return threading.currentThread().getName()
+    return threading.current_thread().name
 
 
 def getLogger(name=None, fname=False, clsname=False, fancyrecord=None):
@@ -577,8 +577,8 @@ def logToFile(filename, enable=True, filehandler=None, name=None, max_bytes=MAX_
         'mode': 'a',
         'maxBytes': max_bytes,
         'backupCount': backup_count,
+        'encoding': 'utf-8',
     }
-    handleropts['encoding'] = 'utf-8'
     # logging to a file is going to create the file later on, so let's try to be helpful and create the path if needed
     directory = os.path.dirname(filename)
     if not os.path.exists(directory):
@@ -783,7 +783,7 @@ def getAllExistingLoggers():
     """
     # not-so-well documented manager (in 2.6 and later)
     # return list of (name,logger) tuple
-    return [x for x in logging.Logger.manager.loggerDict.items()] + [(logging.root.name, logging.root)]
+    return list(logging.Logger.manager.loggerDict.items()) + [(logging.root.name, logging.root)]
 
 
 def getAllNonFancyloggers():
diff --git a/easybuild/base/wrapper.py b/easybuild/base/wrapper.py
@@ -39,7 +39,7 @@ def proxy(self, *args):  # pylint:disable=unused-argument
         # create proxies for wrapped object's double-underscore attributes
         type.__init__(cls, name, bases, dct)
         if cls.__wraps__:
-            ignore = set("__%s__" % n for n in cls.__ignore__.split())
+            ignore = {"__%s__" % n for n in cls.__ignore__.split()}
             for name in dir(cls.__wraps__):
                 if name.startswith("__"):
                     if name not in ignore and name not in dct:
diff --git a/easybuild/framework/easyconfig/easyconfig.py b/easybuild/framework/easyconfig/easyconfig.py
@@ -261,7 +261,7 @@ def det_subtoolchain_version(current_tc, subtoolchain_names, optional_toolchains
 
     for subtoolchain_name in subtoolchain_names:
 
-        uniq_subtc_versions = set([subtc['version'] for subtc in cands if subtc['name'] == subtoolchain_name])
+        uniq_subtc_versions = {subtc['version'] for subtc in cands if subtc['name'] == subtoolchain_name}
 
         # system toolchain: bottom of the hierarchy
         if is_system_toolchain(subtoolchain_name):
@@ -318,8 +318,8 @@ def get_toolchain_hierarchy(parent_toolchain, incl_capabilities=False):
     # obtain list of all possible subtoolchains
     _, all_tc_classes = search_toolchain('')
     subtoolchains = {tc_class.NAME: getattr(tc_class, 'SUBTOOLCHAIN', None) for tc_class in all_tc_classes}
-    optional_toolchains = set(tc_class.NAME for tc_class in all_tc_classes if getattr(tc_class, 'OPTIONAL', False))
-    composite_toolchains = set(tc_class.NAME for tc_class in all_tc_classes if len(tc_class.__bases__) > 1)
+    optional_toolchains = {tc_class.NAME for tc_class in all_tc_classes if getattr(tc_class, 'OPTIONAL', False)}
+    composite_toolchains = {tc_class.NAME for tc_class in all_tc_classes if len(tc_class.__bases__) > 1}
 
     # the parent toolchain is at the top of the hierarchy,
     # we need a copy so that adding capabilities (below) doesn't affect the original object
diff --git a/easybuild/framework/easyconfig/types.py b/easybuild/framework/easyconfig/types.py
@@ -541,7 +541,7 @@ def _to_checksum(checksum, list_level=0, allow_dict=True):
                 # When we already are in a tuple no further recursion is allowed -> set list_level very high
                 return tuple(_to_checksum(x, list_level=99, allow_dict=allow_dict) for x in checksum)
             else:
-                return list(_to_checksum(x, list_level=list_level+1, allow_dict=allow_dict) for x in checksum)
+                return [_to_checksum(x, list_level=list_level+1, allow_dict=allow_dict) for x in checksum]
     elif isinstance(checksum, dict) and allow_dict:
         return {key: _to_checksum(value, allow_dict=False) for key, value in checksum.items()}
 
diff --git a/easybuild/tools/docs.py b/easybuild/tools/docs.py
@@ -1468,7 +1468,7 @@ def gen_easyblock_doc_section_md(eb_class, path_to_examples, common_params, doc_
 
         table_titles = ['easyconfig parameter', 'description']
         table_values = [
-            [opt for opt in common_params[classname]],
+            common_params[classname],
             [DEFAULT_CONFIG[opt][1] for opt in common_params[classname]],
         ]
 
@@ -1556,7 +1556,7 @@ def gen_easyblock_doc_section_rst(eb_class, path_to_examples, common_params, doc
 
         table_titles = ['easyconfig parameter', 'description']
         table_values = [
-            [opt for opt in common_params[classname]],
+            common_params[classname],
             [DEFAULT_CONFIG[opt][1] for opt in common_params[classname]],
         ]
 
diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py
@@ -786,7 +786,7 @@ def parse_http_header_fields_urlpat(arg, urlpat=None, header=None, urlpat_header
             if urlpat in urlpat_headers.keys():
                 urlpat_headers[urlpat].append(argline)  # add headers to the list
             else:
-                urlpat_headers[urlpat] = list([argline])  # new list headers for this urlpat
+                urlpat_headers[urlpat] = [argline]  # new list headers for this urlpat
         else:
             _log.warning("Non-empty argument to http-header-fields-urlpat ignored (missing URL pattern)")
 
@@ -838,7 +838,7 @@ def download_file(filename, url, path, forced=False, trace=True, max_attempts=No
     # parse option HTTP header fields for URLs containing a pattern
     http_header_fields_urlpat = build_option('http_header_fields_urlpat')
     # compile a dict full of {urlpat: [header, list]}
-    urlpat_headers = dict()
+    urlpat_headers = {}
     if http_header_fields_urlpat is not None:
         # there may be multiple options given, parse them all, while updating urlpat_headers
         for arg in http_header_fields_urlpat:
@@ -2368,7 +2368,7 @@ def encode_string(name):
     """
 
     # do the character remapping, return same char by default
-    result = ''.join(map(lambda x: STRING_ENCODING_CHARMAP.get(x, x), name))
+    result = ''.join(STRING_ENCODING_CHARMAP.get(x, x) for x in name)
     return result
 
 
diff --git a/easybuild/tools/modules.py b/easybuild/tools/modules.py
@@ -1289,7 +1289,7 @@ def run_module(self, *args, **kwargs):
             # keep track of current values of select env vars, so we can correct the adjusted values below
             # Identical to `{key: os.environ.get(key, '').split(os.pathsep)[::-1] for key in LD_ENV_VAR_KEYS}`
             # but Python 2 treats that as a local function and refused the `exec` below
-            prev_ld_values = dict([(key, os.environ.get(key, '').split(os.pathsep)[::-1]) for key in LD_ENV_VAR_KEYS])
+            prev_ld_values = {key: os.environ.get(key, '').split(os.pathsep)[::-1] for key in LD_ENV_VAR_KEYS}
 
             # Change the environment
             try:
diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py
@@ -128,15 +128,15 @@ def terminal_supports_colors(stream):
 
 XDG_CONFIG_HOME = os.environ.get('XDG_CONFIG_HOME', os.path.join(os.path.expanduser('~'), ".config"))
 XDG_CONFIG_DIRS = os.environ.get('XDG_CONFIG_DIRS', '/etc/xdg').split(os.pathsep)
-DEFAULT_SYS_CFGFILES = [[f for f in sorted(glob.glob(os.path.join(d, 'easybuild.d', '*.cfg')))]
+DEFAULT_SYS_CFGFILES = [sorted(glob.glob(os.path.join(d, 'easybuild.d', '*.cfg')))
                         for d in XDG_CONFIG_DIRS]
 DEFAULT_USER_CFGFILE = os.path.join(XDG_CONFIG_HOME, 'easybuild', 'config.cfg')
 
 DEFAULT_LIST_PR_STATE = GITHUB_PR_STATE_OPEN
 DEFAULT_LIST_PR_ORDER = GITHUB_PR_ORDER_CREATED
 DEFAULT_LIST_PR_DIREC = GITHUB_PR_DIRECTION_DESC
 
-RPATH_DEFAULT = False if get_os_type() == DARWIN else True
+RPATH_DEFAULT = get_os_type() != DARWIN
 
 _log = fancylogger.getLogger('options', fname=False)
 
@@ -1135,7 +1135,7 @@ def _postprocess_close_pr_reasons(self):
             )
 
         reasons = self.options.close_pr_reasons.split(',')
-        if any([reason not in VALID_CLOSE_PR_REASONS.keys() for reason in reasons]):
+        if any(reason not in VALID_CLOSE_PR_REASONS for reason in reasons):
             raise EasyBuildError(
                 "Argument to --close-pr_reasons must be a comma separated list of valid reasons among %s",
                 VALID_CLOSE_PR_REASONS.keys(), exit_code=EasyBuildExit.OPTION_ERROR
@@ -1561,7 +1561,7 @@ def show_system_info(self):
             "* software:",
             "  -> glibc version: %s" % system_info['glibc_version'],
             "  -> Python binary: %s" % sys.executable,
-            "  -> Python version: %s" % sys.version.split(' ')[0],
+            "  -> Python version: %s" % sys.version.split(' ', maxsplit=1)[0],
         ])
 
         return '\n'.join(lines)
@@ -1749,7 +1749,7 @@ def check_included_multiple(included_easyblocks_from, source):
         if options.include_easyblocks:
             # check if you are including the same easyblock twice
             included_paths = expand_glob_paths(options.include_easyblocks)
-            included_easyblocks = set([os.path.basename(eb) for eb in included_paths])
+            included_easyblocks = {os.path.basename(eb) for eb in included_paths}
 
         if options.include_easyblocks_from_pr:
             try:
@@ -1762,7 +1762,7 @@ def check_included_multiple(included_easyblocks_from, source):
 
             for easyblock_pr in easyblock_prs:
                 easyblocks_from_pr = fetch_easyblocks_from_pr(easyblock_pr)
-                included_from_pr = set([os.path.basename(eb) for eb in easyblocks_from_pr])
+                included_from_pr = {os.path.basename(eb) for eb in easyblocks_from_pr}
 
                 if options.include_easyblocks:
                     check_included_multiple(included_from_pr, "PR #%s" % easyblock_pr)
@@ -1776,7 +1776,7 @@ def check_included_multiple(included_easyblocks_from, source):
         easyblock_commit = options.include_easyblocks_from_commit
         if easyblock_commit:
             easyblocks_from_commit = fetch_easyblocks_from_commit(easyblock_commit)
-            included_from_commit = set([os.path.basename(eb) for eb in easyblocks_from_commit])
+            included_from_commit = {os.path.basename(eb) for eb in easyblocks_from_commit}
 
             if options.include_easyblocks:
                 check_included_multiple(included_from_commit, "commit %s" % easyblock_commit)
diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py
@@ -277,7 +277,7 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_
     end_time = strftime(time_format, end_time)
     test_report.extend(["#### Time info", " * start: %s" % start_time, " * end: %s" % end_time, ""])
 
-    eb_config = [x for x in sorted(init_session_state['easybuild_configuration'])]
+    eb_config = sorted(init_session_state['easybuild_configuration'])
     test_report.extend([
         "#### EasyBuild info",
         " * easybuild-framework version: %s" % FRAMEWORK_VERSION,
diff --git a/easybuild/tools/toolchain/toolchain.py b/easybuild/tools/toolchain/toolchain.py
@@ -597,7 +597,7 @@ def definition(self):
 
     def is_dep_in_toolchain_module(self, name):
         """Check whether a specific software name is listed as a dependency in the module for this toolchain."""
-        return any(map(lambda m: self.mns.is_short_modname_for(m, name), self.toolchain_dep_mods))
+        return any(self.mns.is_short_modname_for(m, name) for m in self.toolchain_dep_mods)
 
     def _simulated_load_dependency_module(self, name, version, metadata, verbose=False):
         """
@@ -752,7 +752,7 @@ def _verify_toolchain(self):
         self.log.debug("List of toolchain dependencies from toolchain module: %s", self.toolchain_dep_mods)
 
         # only retain names of toolchain elements, excluding toolchain name
-        toolchain_definition = set([e for es in self.definition().values() for e in es if not e == self.name])
+        toolchain_definition = {e for es in self.definition().values() for e in es if not e == self.name}
 
         # filter out optional toolchain elements if they're not used in the module
         for elem_name in toolchain_definition.copy():
diff --git a/easybuild/tools/variables.py b/easybuild/tools/variables.py
@@ -370,8 +370,8 @@ def sanitize(self):
                         # keep last
                         to_remove.extend(all_idx[:-1])
 
-            to_remove = sorted(list(set(to_remove)), reverse=True)
-            self.log.devel("sanitize: to_remove in %s %s", self.__repr__(), to_remove)
+            to_remove = sorted(set(to_remove), reverse=True)
+            self.log.devel("sanitize: to_remove in %s %s", repr(self), to_remove)
             for idx in to_remove:
                 del self[idx]
 
@@ -388,7 +388,7 @@ def sanitize(self):
                     self.log.devel("sanitize: JOIN_BEGIN_END idx %s joining %s and %s", idx, self[idx], self[idx - 1])
                     self[idx - 1].extend(self[idx])
                     to_remove.append(idx)  # remove current el
-            to_remove = sorted(list(set(to_remove)), reverse=True)
+            to_remove = sorted(set(to_remove), reverse=True)
             for idx in to_remove:
                 del self[idx]
 
diff --git a/setup.cfg b/setup.cfg
@@ -9,14 +9,10 @@ requires = environment-modules, bash, python >= 2.6, python < 3
 
 [flake8]
 max-line-length = 120
+# C4: Comprehensions
+extend-select = C4
 
-# Hound CI runs with Python 3 (no way around it),
-# so we need to specify some Python 2 builtins to avoid that it complains about them
-# cfr. https://stackoverflow.com/questions/47427916/how-to-config-hound-ci-to-support-python2-7
-builtins =
-    basestring,
-    reduce
-
-# ignore "Black would make changes" produced by flake8-black
+# BLK100: "Black would make changes"
 # see also https://github.com/houndci/hound/issues/1769
-extend-ignore = BLK100
+# G002-G004,G200: Logging statement uses '%', '+', f-string, exception
+extend-ignore = BLK100,G002,G003,G004,G200
diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py
@@ -3558,7 +3558,7 @@ def test_arch_specific_sanity_check(self):
         eb = EasyBlock(ec)
         paths, _, _ = eb._sanity_check_step_common(None, None)
 
-        self.assertEqual(set(paths.keys()), set(('files', 'dirs')))
+        self.assertEqual(set(paths.keys()), {'files', 'dirs'})
         self.assertEqual(paths['files'], ['correct.a', 'default.a'])
         self.assertEqual(paths['dirs'], [('correct', 'alternative')])
 
diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py
@@ -2501,12 +2501,12 @@ def eval_quoted_string(quoted_val, val):
             Helper function to sanity check we can use the quoted string in Python contexts.
             Returns the evaluated (i.e. unquoted) string
             """
-            scope = dict()
+            scope = {}
             try:
                 # this is needlessly complicated because we can't use 'exec' here without potentially running
                 # into a SyntaxError bug in old Python 2.7 versions (for example when running the tests in CentOS 7.9)
                 # cfr. https://stackoverflow.com/questions/4484872/why-doesnt-exec-work-in-a-function-with-a-subfunction
-                eval(compile('res = %s' % quoted_val, '<string>', 'exec'), dict(), scope)
+                eval(compile('res = %s' % quoted_val, '<string>', 'exec'), {}, scope)
             except Exception as err:  # pylint: disable=broad-except
                 self.fail('Failed to evaluate %s (from %s): %s' % (quoted_val, val, err))
             return scope['res']
@@ -3942,7 +3942,7 @@ def test_det_subtoolchain_version(self):
         """Test det_subtoolchain_version function"""
         _, all_tc_classes = search_toolchain('')
         subtoolchains = {tc_class.NAME: getattr(tc_class, 'SUBTOOLCHAIN', None) for tc_class in all_tc_classes}
-        optional_toolchains = set(tc_class.NAME for tc_class in all_tc_classes if getattr(tc_class, 'OPTIONAL', False))
+        optional_toolchains = {tc_class.NAME for tc_class in all_tc_classes if getattr(tc_class, 'OPTIONAL', False)}
 
         current_tc = {'name': 'fosscuda', 'version': '2018a'}
         # missing gompic and double golfc should both give exceptions
diff --git a/test/framework/filetools.py b/test/framework/filetools.py
@@ -3752,7 +3752,7 @@ def test_locks(self):
         ft.clean_up_locks()
 
         ft.create_lock(lock_name)
-        self.assertEqual(ft.global_lock_names, set([lock_name]))
+        self.assertEqual(ft.global_lock_names, {lock_name})
         self.assertEqual(os.listdir(locks_dir), [lock_name + '.lock'])
 
         ft.clean_up_locks()
diff --git a/test/framework/github.py b/test/framework/github.py
@@ -211,7 +211,7 @@ def test_github_walk(self):
                 ('a_directory', ['a_subdirectory'], ['a_file.txt']),
                 ('a_directory/a_subdirectory', [], ['a_file.txt']), ('second_dir', [], ['a_file.txt']),
             ]
-            self.assertEqual([x for x in self.ghfs.walk(None)], expected)
+            self.assertEqual(list(self.ghfs.walk(None)), expected)
         except IOError:
             pass
 
diff --git a/test/framework/options.py b/test/framework/options.py
@@ -5668,7 +5668,7 @@ def test_zip_logs(self):
             logs = glob.glob(os.path.join(toy_eb_install_dir, 'easybuild-toy-0.0*log*'))
             self.assertEqual(len(logs), 1, "Found exactly 1 log file in %s: %s" % (toy_eb_install_dir, logs))
 
-            zip_logs_arg = zip_logs.split('=')[-1]
+            zip_logs_arg = zip_logs.rsplit('=', maxsplit=1)[-1]
             if zip_logs == '--zip-logs' or zip_logs_arg == 'gzip':
                 ext = 'log.gz'
             elif zip_logs_arg == 'bzip2':

Original file line number	Diff line number	Diff line change
`@@ -1468,7 +1468,7 @@ def gen_easyblock_doc_section_md(eb_class, path_to_examples, common_params, doc_`
`1468`	`1468`
`1469`	`1469`	`table_titles = ['easyconfig parameter', 'description']`
`1470`	`1470`	`table_values = [`
`1471`		`- [opt for opt in common_params[classname]],`
	`1471`	`+ common_params[classname],`
`1472`	`1472`	`[DEFAULT_CONFIG[opt][1] for opt in common_params[classname]],`
`1473`	`1473`	`]`
`1474`	`1474`
`@@ -1556,7 +1556,7 @@ def gen_easyblock_doc_section_rst(eb_class, path_to_examples, common_params, doc`
`1556`	`1556`
`1557`	`1557`	`table_titles = ['easyconfig parameter', 'description']`
`1558`	`1558`	`table_values = [`
`1559`		`- [opt for opt in common_params[classname]],`
	`1559`	`+ common_params[classname],`
`1560`	`1560`	`[DEFAULT_CONFIG[opt][1] for opt in common_params[classname]],`
`1561`	`1561`	`]`
`1562`	`1562`