Merge pull request #1449 from brian-team/flake8-precommit

flake8 precommit hook

.devcontainer/dev-requirements.txt

@@ -7,3 +7,5 @@ pre-commit == 2.20.*
 black == 22.10.0
 isort == 5.10.1
 pyupgrade == 3.2.3
+flake8 == 4.0.1
+flake8-bugbear == 21.11.29

.devcontainer/devcontainer.json

@@ -19,13 +19,13 @@
 	"settings": {
 		"python.defaultInterpreterPath": "/usr/local/bin/python",
 		"python.linting.enabled": true,
-		"python.linting.pylintEnabled": true,
+		"python.linting.flake8Enabled": true,
 		"python.formatting.provider": "black",
 		"python.formatting.autopep8Path": "/usr/local/py-utils/bin/autopep8",
 		"python.formatting.blackPath": "black",
 		"python.formatting.yapfPath": "/usr/local/py-utils/bin/yapf",
 		"python.linting.banditPath": "/usr/local/py-utils/bin/bandit",
-		"python.linting.flake8Path": "/usr/local/py-utils/bin/flake8",
+		"python.linting.flake8Path": "flake8",
 		"python.linting.mypyPath": "/usr/local/py-utils/bin/mypy",
 		"python.linting.pycodestylePath": "/usr/local/py-utils/bin/pycodestyle",
 		"python.linting.pydocstylePath": "/usr/local/py-utils/bin/pydocstyle",
@@ -53,7 +53,7 @@
 
 	// Use 'postCreateCommand' to run commands after the container is created.
 	// "postCreateCommand": "pip3 install --user -r requirements.txt",
-	"postCreateCommand": "pre-commit install && pip3 install --user -e '.[test,doc]' && pip3 freeze > .devcontainer/frozen-requirements.txt",
+	"postCreateCommand": "pre-commit install && pip3 install --user -e '.[test,docs]' && pip3 freeze > .devcontainer/frozen-requirements.txt",
 
 	// Comment out to connect as root instead. More info: https://aka.ms/vscode-remote/containers/non-root.
 	"remoteUser": "vscode",

.git-blame-ignore-revs

@@ -12,3 +12,5 @@
 67bf6d3760fa3fb8b3aa121b1b972d6cf36ec048
 # Update syntax to Python 3.8 with pyupgrade
 28b02c51545298cb9a76d8295e64a5df391b9207
+# Fix a number of issues flagged by flake8
+0344b005e6ffd232f55e69621630ef01876bc0fb

.pre-commit-config.yaml

@@ -10,6 +10,14 @@ repos:
         args: [--py38-plus]
         exclude: '^brian2/_version.py$'
         files: '^brian2/.*\.pyi?$'
+  - repo: https://github.com/pycqa/flake8
+    rev: 4.0.1
+    hooks:
+      - id: flake8
+        # We exclude all __init__.py files, since we use wildcard imports, etc.
+        exclude: '^brian2/_version[.]py$|^brian2/tests/.*$|^brian2/.*__init__[.]py$'
+        files: '^brian2/.*\.pyi?$'
+        additional_dependencies: ['flake8-bugbear==21.11.29']
   - repo: https://github.com/pycqa/isort
     rev: 5.10.1
     hooks:

brian2/codegen/_prefs.py

@@ -17,16 +17,16 @@
         default="auto",
         docs="""
         Default target for code generation.
-        
+
         Can be a string, in which case it should be one of:
-        
+
         * ``'auto'`` the default, automatically chose the best code generation
           target available.
         * ``'cython'``, uses the Cython package to generate C++ code. Needs a
           working installation of Cython and a C++ compiler.
         * ``'numpy'`` works on all platforms and doesn't need a C compiler but
           is often less efficient.
-        
+
         Or it can be a ``CodeObject`` class.
         """,
         validator=lambda target: isinstance(target, str)

brian2/codegen/codeobject.py

@@ -164,7 +164,7 @@ def _error_msg(code, name):
     Little helper function for error messages.
     """
     error_msg = f"Error generating code for code object '{name}' "
-    code_lines = [l for l in code.split("\n") if len(l.strip())]
+    code_lines = [line for line in code.split("\n") if len(line.strip())]
     # If the abstract code is only one line, display it in full
     if len(code_lines) <= 1:
         error_msg += f"from this abstract code: '{code_lines[0]}'\n"
@@ -195,7 +195,7 @@ def check_compiler_kwds(compiler_kwds, accepted_kwds, target):
     ValueError
         If a compiler keyword is not understood
     """
-    for key, value in compiler_kwds.items():
+    for key in compiler_kwds:
         if key not in accepted_kwds:
             formatted_kwds = ", ".join(f"'{kw}'" for kw in accepted_kwds)
             raise ValueError(
@@ -465,7 +465,7 @@ def create_runner_codeobj(
     # Add the indices needed by the variables
     for varname in list(variables):
         var_index = all_variable_indices[varname]
-        if not var_index in ("_idx", "0"):
+        if var_index not in ("_idx", "0"):
             variables[var_index] = all_variables[var_index]
 
     return device.code_object(

brian2/codegen/cpp_prefs.py

@@ -253,12 +253,13 @@
         default="",
         docs="""
         MSVC architecture name (or use system architectue by default).
-        
+
         Could take values such as x86, amd64, etc.
         """,
     ),
 )
 
+
 # check whether compiler supports a flag
 # Adapted from https://github.com/pybind/pybind11/
 def _determine_flag_compatibility(compiler, flagname):

brian2/codegen/generators/GSL_generator.py

@@ -18,11 +18,15 @@
 from brian2.core.variables import ArrayVariable, AuxiliaryVariable, Constant
 from brian2.parsing.statements import parse_statement
 from brian2.units.fundamentalunits import fail_for_dimension_mismatch
+from brian2.utils.logger import get_logger
 from brian2.utils.stringtools import get_identifiers, word_substitute
 
 __all__ = ["GSLCodeGenerator", "GSLCPPCodeGenerator", "GSLCythonCodeGenerator"]
 
 
+logger = get_logger(__name__)
+
+
 def valid_gsl_dir(val):
     """
     Validate given string to be path containing required GSL files.
@@ -493,8 +497,8 @@ def scale_array_code(self, diff_vars, method_options):
             diff_scale = {}
             for var, error in list(abs_per_var.items()):
                 # first do some checks on input
-                if not var in diff_vars:
-                    if not var in self.variables:
+                if var not in diff_vars:
+                    if var not in self.variables:
                         raise KeyError(
                             "absolute_error specified for variable that "
                             f"does not exist: {var}"
@@ -554,19 +558,14 @@ def find_undefined_variables(self, statements):
         variables = self.variables
         other_variables = {}
         for statement in statements:
-            var, op, expr, comment = (
-                statement.var,
-                statement.op,
-                statement.expr,
-                statement.comment,
-            )
+            var = statement.var
             if var not in variables:
                 other_variables[var] = AuxiliaryVariable(var, dtype=statement.dtype)
         return other_variables
 
     def find_used_variables(self, statements, other_variables):
         """
-        Find all the variables used in the right hand side of the given
+        Find all the variables used on the right hand side of the given
         expressions.
 
         Parameters
@@ -583,12 +582,7 @@ def find_used_variables(self, statements, other_variables):
         variables = self.variables
         used_variables = {}
         for statement in statements:
-            lhs, op, rhs, comment = (
-                statement.var,
-                statement.op,
-                statement.expr,
-                statement.comment,
-            )
+            rhs = statement.expr
             for var in get_identifiers(rhs):
                 if var in self.function_names:
                     continue
@@ -993,9 +987,9 @@ def translate(
         # so that _dataholder holds diff_vars as well, even if they don't occur
         # in the actual statements
         for var in list(diff_vars.keys()):
-            if not var in variables_in_vector:
+            if var not in variables_in_vector:
                 variables_in_vector[var] = self.variables[var]
-        # lets keep track of the variables that eventually need to be added to
+        # let's keep track of the variables that eventually need to be added to
         # the _GSL_dataholder somehow
         self.variables_to_be_processed = list(variables_in_vector.keys())
 

brian2/codegen/generators/cpp_generator.py

@@ -66,7 +66,7 @@ def c_data_type(dtype):
         default="__restrict",
         docs="""
         The keyword used for the given compiler to declare pointers as restricted.
-        
+
         This keyword is different on different compilers, the default works for
         gcc and MSVS.
         """,
@@ -75,15 +75,15 @@ def c_data_type(dtype):
         default=False,
         docs="""
         Adds code to flush denormals to zero.
-        
+
         The code is gcc and architecture specific, so may not compile on all
         platforms. The code, for reference is::
 
             #define CSR_FLUSH_TO_ZERO         (1 << 15)
             unsigned csr = __builtin_ia32_stmxcsr();
             csr |= CSR_FLUSH_TO_ZERO;
             __builtin_ia32_ldmxcsr(csr);
-            
+
         Found at `<http://stackoverflow.com/questions/2487653/avoiding-denormal-values-in-c>`_.
         """,
     ),
@@ -449,7 +449,7 @@ def determine_keywords(self):
         from brian2.devices.device import get_device
 
         device = get_device()
-        for varname, var in self.variables.items():
+        for var in self.variables.values():
             if isinstance(var, ArrayVariable):
                 # This is the "true" array name, not the restricted pointer.
                 array_name = device.get_array_name(var)
@@ -564,12 +564,12 @@ def determine_keywords(self):
         template <typename T>
         static inline T _clip(const T value, const double a_min, const double a_max)
         {
-	        if (value < a_min)
-	            return a_min;
-	        if (value > a_max)
-	            return a_max;
-	        return value;
-	    }
+            if (value < a_min)
+                return a_min;
+            if (value > a_max)
+                return a_max;
+            return value;
+        }
         """
 DEFAULT_FUNCTIONS["clip"].implementations.add_implementation(
     CPPCodeGenerator, code=clip_code, name="_clip"
@@ -587,7 +587,7 @@ def determine_keywords(self):
 timestep_code = """
 static inline int64_t _timestep(double t, double dt)
 {
-    return (int64_t)((t + 1e-3*dt)/dt); 
+    return (int64_t)((t + 1e-3*dt)/dt);
 }
 """
 DEFAULT_FUNCTIONS["timestep"].implementations.add_implementation(

brian2/codegen/generators/cython_generator.py

@@ -159,11 +159,10 @@ def translate_to_read_arrays(self, read, indices):
     def translate_to_statements(self, statements, conditional_write_vars):
         lines = []
         for stmt in statements:
-            if stmt.op == ":=" and not stmt.var in self.variables:
+            if stmt.op == ":=" and stmt.var not in self.variables:
                 self.temporary_vars.add((stmt.var, stmt.dtype))
             line = self.translate_statement(stmt)
             if stmt.var in conditional_write_vars:
-                subs = {}
                 condvar = conditional_write_vars[stmt.var]
                 lines.append(f"if {condvar}:")
                 lines.append(indent(line))

brian2/codegen/generators/numpy_generator.py

@@ -242,7 +242,7 @@ def read_arrays(self, read, write, indices, variables, variable_indices):
             #                line = '{varname} = {array_name}.take({index})'
             #            line = line.format(varname=varname, array_name=self.get_array_name(var), index=index)
             line = f"{varname} = {self.get_array_name(var)}"
-            if not index in self.iterate_all:
+            if index not in self.iterate_all:
                 line += f"[{index}]"
             elif varname in write:
                 # avoid potential issues with aliased variables, see github #259
@@ -332,7 +332,7 @@ def translate_one_statement_sequence(self, statements, scalar=False):
 
     def determine_keywords(self):
         try:
-            import scipy
+            import scipy  # noqa: F401
 
             scipy_available = True
         except ImportError:
@@ -366,6 +366,7 @@ def determine_keywords(self):
         NumpyCodeGenerator, code=func
     )
 
+
 # Functions that are implemented in a somewhat special way
 def randn_func(vectorisation_idx):
     try:

brian2/codegen/runtime/GSLcython_rt/GSLcython_rt.py

@@ -56,4 +56,4 @@ def compile(self):
                 "\nIf GSL is installed but Python cannot find the correct files, it is "
                 "also possible to give the gsl directory manually by specifying "
                 "prefs.GSL.directory = ..."
-            )
+            ) from err

brian2/core/base.py

@@ -227,49 +227,49 @@ def run(self):
     contained_objects = property(
         fget=lambda self: self._contained_objects,
         doc="""
-         The list of objects contained within the `BrianObject`.
-         
-         When a `BrianObject` is added to a `Network`, its contained objects will
-         be added as well. This allows for compound objects which contain
-         a mini-network structure.
-         
-         Note that this attribute cannot be set directly, you need to modify
-         the underlying list, e.g. ``obj.contained_objects.extend([A, B])``.
-         """,
+        The list of objects contained within the `BrianObject`.
+
+        When a `BrianObject` is added to a `Network`, its contained objects will
+        be added as well. This allows for compound objects which contain
+        a mini-network structure.
+
+        Note that this attribute cannot be set directly, you need to modify
+        the underlying list, e.g. ``obj.contained_objects.extend([A, B])``.
+        """,
     )
 
     code_objects = property(
         fget=lambda self: self._code_objects,
         doc="""
-         The list of `CodeObject` contained within the `BrianObject`.
-         
-         TODO: more details.
-                  
-         Note that this attribute cannot be set directly, you need to modify
-         the underlying list, e.g. ``obj.code_objects.extend([A, B])``.
-         """,
+        The list of `CodeObject` contained within the `BrianObject`.
+
+        TODO: more details.
+
+        Note that this attribute cannot be set directly, you need to modify
+        the underlying list, e.g. ``obj.code_objects.extend([A, B])``.
+        """,
     )
 
     updaters = property(
         fget=lambda self: self._updaters,
         doc="""
-         The list of `Updater` that define the runtime behaviour of this object.
-         
-         TODO: more details.
-                  
-         Note that this attribute cannot be set directly, you need to modify
-         the underlying list, e.g. ``obj.updaters.extend([A, B])``.
-         """,
+        The list of `Updater` that define the runtime behaviour of this object.
+
+        TODO: more details.
+
+        Note that this attribute cannot be set directly, you need to modify
+        the underlying list, e.g. ``obj.updaters.extend([A, B])``.
+        """,
     )
 
     clock = property(
         fget=lambda self: self._clock,
         doc="""
-                     The `Clock` determining when the object should be updated.
-                     
-                     Note that this cannot be changed after the object is
-                     created.
-                     """,
+        The `Clock` determining when the object should be updated.
+
+        Note that this cannot be changed after the object is
+        created.
+        """,
     )
 
     def _set_active(self, val):
@@ -282,13 +282,13 @@ def _set_active(self, val):
         fget=lambda self: self._active,
         fset=_set_active,
         doc="""
-                        Whether or not the object should be run.
-                        
-                        Inactive objects will not have their `update`
-                        method called in `Network.run`. Note that setting or
-                        unsetting the `active` attribute will set or unset
-                        it for all `contained_objects`. 
-                        """,
+        Whether or not the object should be run.
+
+        Inactive objects will not have their `update`
+        method called in `Network.run`. Note that setting or
+        unsetting the `active` attribute will set or unset
+        it for all `contained_objects`.
+        """,
     )
 
     def __repr__(self):