Refactor global pylint checks (#788)

* Upgrade pylint version

* Fix remaining pylint errors in `randprocs` (excluding `markov`)

* Fix pylint errors in `_config.py`

* Add a global linting pass ignoring the problematic subpackages

* Force parameters, exceptions, yields and return types to be documented in the docstring

* Fix pylint messages in `utils`

* Disable new messages in per-package linting passes

* Remove the sync check between global and local linting passes

* Bugfix

Author: marvinpfoertner

.github/workflows/linting.yml

@@ -36,13 +36,3 @@ jobs:
         run: pip install tox
       - name: Run pylint through tox
         run: tox -e pylint
-
-  pylint-tox-config-check:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-python@v2
-        with:
-          python-version: 3.8
-      - name: Check if the list of disabled messages in the local and global linting passes in ./tox.ini are correctly synchronized
-        run: python .github/workflows/pylint_check.py

.github/workflows/pylint_check.py

@@ -1,4 +1,4 @@
 # Dependencies for code linting
 
-pylint == 2.9.*
+pylint~=2.16.2
 # mypy

linting-requirements.txt

@@ -163,18 +163,17 @@ load-plugins = [
     "pylint.extensions.docstyle",
     "pylint.extensions.overlapping_exceptions",
     "pylint.extensions.mccabe",
+    "pylint.extensions.no_self_use",
 ]
 
 [tool.pylint.messages_control]
 disable = [
-    # Exceptions suggested by Black:
-    # https://github.com/psf/black/blob/7f75fe3669ebf0627b1b0476a6d02047e909b959/docs/compatible_configs.md#black-compatible-configurations
-    "bad-continuation",
-    "bad-whitespace",
     # We allow TODO comments in the following format: `# TODO (#[ISSUE NUMBER]): This needs to be done.`
     "fixme",
     # We want to use "mathematical notation" to name some of our variables, e.g. `A` for matrices
     "invalid-name",
+    # Assigning lambda expressions to a variable is sometimes useful, e.g. for defining `LambdaLinearOperator`s
+    "unnecessary-lambda-assignment",
     # Temporary ignore, see https://github.com/probabilistic-numerics/probnum/discussions/470#discussioncomment-1998097 for an explanation
     "missing-return-doc",
     "missing-yield-doc",
@@ -183,13 +182,23 @@ disable = [
 [tool.pylint.format]
 max-line-length = "88"
 
+[tool.pylint.parameter_documentation]
+accept-no-param-doc = false
+accept-no-raise-doc = false
+accept-no-return-doc = false
+accept-no-yields-doc = false
+
 [tool.pylint.design]
 max-args = 10
 max-complexity = 14
 max-locals = 20
 
 [tool.pylint.similarities]
-ignore-imports = "yes"
+ignore-comments = true
+ignore-docstrings = true
+ignore-imports = true
+ignore-signatures = true
+min-similarity-lines = 4
 
 ################################################################################
 # Formatting Configuration                                                     #

pyproject.toml

@@ -1,3 +1,5 @@
+"""ProbNum library configuration"""
+
 import contextlib
 import dataclasses
 from typing import Any
@@ -35,6 +37,9 @@ class Configuration:
 
     @dataclasses.dataclass
     class Option:
+        """Representation of a single configuration option as a key-value pair with a
+        default value and a description string for documentation purposes."""
+
         name: str
         default_value: Any
         description: str
@@ -49,7 +54,7 @@ def __init__(self) -> None:
         # This is the equivalent of `self._options_registry = dict()`.
         # After rewriting the `__setattr__` method, we have to fall back on the
         # `__setattr__` method of the super class.
-        object.__setattr__(self, "_options_registry", dict())
+        object.__setattr__(self, "_options_registry", {})
 
     def __getattr__(self, key: str) -> Any:
         if key not in self._options_registry:
@@ -68,7 +73,7 @@ def __repr__(self) -> str:
     @contextlib.contextmanager
     def __call__(self, **kwargs) -> None:
         """Context manager used to set values of registered config options."""
-        old_options = dict()
+        old_options = {}
 
         for key, value in kwargs.items():
             if key not in self._options_registry:
@@ -96,6 +101,11 @@ def register(self, key: str, default_value: Any, description: str) -> None:
             The default value of the configuration option.
         description:
             A short description of the configuration option and what it controls.
+
+        Raises
+        ------
+        KeyError
+            If the configuration option already exists.
         """
         if key in self._options_registry:
             raise KeyError(
@@ -156,5 +166,9 @@ def register(self, key: str, default_value: Any, description: str) -> None:
 ]
 
 # ... and register the default configuration options.
-for key, default_value, descr in _DEFAULT_CONFIG_OPTIONS:
-    _GLOBAL_CONFIG_SINGLETON.register(key, default_value, descr)
+def _register_defaults():
+    for key, default_value, descr in _DEFAULT_CONFIG_OPTIONS:
+        _GLOBAL_CONFIG_SINGLETON.register(key, default_value, descr)
+
+
+_register_defaults()

src/probnum/_config.py

@@ -7,7 +7,7 @@
 
 
 @pytest.fixture(autouse=True)
-def autoimport_packages(doctest_namespace):
+def autoimport_packages(doctest_namespace):  # pylint: disable=missing-any-param-doc
     """This fixture 'imports' standard packages automatically in order to avoid
     boilerplate code in doctests"""
 

src/probnum/conftest.py

@@ -129,9 +129,8 @@ def __sub__(self, other):
     @functools.singledispatchmethod
     def __mul__(self, other):
         if np.ndim(other) == 0:
-            from ._algebra_fallbacks import (  # pylint: disable=import-outside-toplevel
-                ScaledFunction,
-            )
+            # pylint: disable=import-outside-toplevel,cyclic-import
+            from ._algebra_fallbacks import ScaledFunction
 
             return ScaledFunction(function=self, scalar=other)
 
@@ -140,9 +139,8 @@ def __mul__(self, other):
     @functools.singledispatchmethod
     def __rmul__(self, other):
         if np.ndim(other) == 0:
-            from ._algebra_fallbacks import (  # pylint: disable=import-outside-toplevel
-                ScaledFunction,
-            )
+            # pylint: disable=import-outside-toplevel,cyclic-import
+            from ._algebra_fallbacks import ScaledFunction
 
             return ScaledFunction(function=self, scalar=other)
 

src/probnum/functions/_function.py

@@ -137,10 +137,10 @@ def __neg__(self):
         return SumLinearOperator(*(-summand for summand in self._summands))
 
     def __repr__(self):
-        res = "SumLinearOperator [\n"
-        for s in self._summands:
-            res += f"\t{s}, \n"
-        return res + "]"
+        res = "SumLinearOperator [\n\t"
+        res += ",\n\t".join(repr(summand) for summand in self._summands)
+        res += "\n]"
+        return res
 
     @staticmethod
     def _expand_sum_ops(*summands: LinearOperator) -> Tuple[LinearOperator, ...]:
@@ -230,10 +230,10 @@ def _expand_prod_ops(*factors: LinearOperator) -> Tuple[LinearOperator, ...]:
         return tuple(expanded_factors)
 
     def __repr__(self):
-        res = "ProductLinearOperator [\n"
-        for s in self._factors:
-            res += f"\t{s}, \n"
-        return res + "]"
+        res = "ProductLinearOperator [\n\t"
+        res += ",\n\t".join(repr(factor) for factor in self._factors)
+        res += "\n]"
+        return res
 
     def _solve(self, B: np.ndarray) -> np.ndarray:
         return functools.reduce(lambda b, op: op.solve(b), self._factors, B)

src/probnum/linops/_arithmetic_fallbacks.py

@@ -10,7 +10,7 @@
 import scipy.linalg
 import scipy.sparse
 
-from probnum import config
+from probnum import config  # pylint: disable=cyclic-import
 from probnum.typing import ArrayLike, DTypeLike, ScalarLike, ShapeLike
 import probnum.utils
 
@@ -384,6 +384,13 @@ def todense(self, cache: bool = True) -> np.ndarray:
         This method can be computationally very costly depending on the shape of the
         linear operator. Use with caution.
 
+        Parameters
+        ----------
+        cache
+            If this is set to :data:`True`, then the dense matrix representation will
+            be cached and subsequent calls will return the cached value (even if
+            :code:`dense` is set to :data:`False` in these subsequent calls).
+
         Returns
         -------
         matrix : np.ndarray
@@ -413,7 +420,14 @@ def is_symmetric(self) -> Optional[bool]:
         """Whether the ``LinearOperator`` :math:`L` is symmetric, i.e. :math:`L = L^T`.
 
         If this is ``None``, it is unknown whether the operator is symmetric or not.
-        Only square operators can be symmetric."""
+        Only square operators can be symmetric.
+
+        Raises
+        ------
+        ValueError
+            When setting :attr:`is_symmetric` to :data:`True` on a non-square
+            :class:`LinearOperator`.
+        """
         return self._is_symmetric
 
     @is_symmetric.setter
@@ -458,6 +472,12 @@ def is_positive_definite(self) -> Optional[bool]:
 
         If this is ``None``, it is unknown whether the matrix is positive-definite or
         not. Only symmetric operators can be positive-definite.
+
+        Raises
+        ------
+        ValueError
+            When setting :attr:`is_positive_definite` to :data:`True` while
+            :attr:`is_symmetric` is :data:`False`.
         """
         return self._is_positive_definite
 
@@ -520,7 +540,13 @@ def _eigvals(self) -> np.ndarray:
         return np.linalg.eigvals(self.todense(cache=False))
 
     def eigvals(self) -> np.ndarray:
-        """Eigenvalue spectrum of the linear operator."""
+        """Eigenvalue spectrum of the linear operator.
+
+        Raises
+        ------
+        numpy.linalg.LinAlgError
+            If :meth:`eigvals` is called on a non-square operator.
+        """
         if self._eigvals_cache is None:
             if not self.is_square:
                 raise np.linalg.LinAlgError(
@@ -871,9 +897,8 @@ def _lu_factor(self):
     ####################################################################################
 
     def __neg__(self) -> "LinearOperator":
-        from ._arithmetic import (  # pylint: disable=import-outside-toplevel
-            NegatedLinearOperator,
-        )
+        # pylint: disable=import-outside-toplevel,cyclic-import
+        from ._arithmetic import NegatedLinearOperator
 
         return NegatedLinearOperator(self)
 
@@ -912,6 +937,18 @@ def transpose(self, *axes: Union[int, Tuple[int]]) -> "LinearOperator":
         """Transpose this linear operator.
 
         Can be abbreviated self.T instead of self.transpose().
+
+        Parameters
+        ----------
+        *axes
+            Permutation of the axes of the :class:`LinearOperator`.
+
+        Raises
+        ------
+        ValueError
+            If the given axis indices do not constitute a valid permutation of the axes.
+        numpy.AxisError
+            If the axis indices are out of bounds.
         """
         if len(axes) > 0:
             if len(axes) == 1 and isinstance(axes[0], tuple):
@@ -1167,7 +1204,8 @@ def __matmul__(
 
             return y
 
-        from ._arithmetic import matmul  # pylint: disable=import-outside-toplevel
+        # pylint: disable=import-outside-toplevel,cyclic-import
+        from ._arithmetic import matmul
 
         return matmul(self, other)
 
@@ -1193,7 +1231,8 @@ def __rmatmul__(
 
             return y
 
-        from ._arithmetic import matmul  # pylint: disable=import-outside-toplevel
+        # pylint: disable=import-outside-toplevel,cyclic-import
+        from ._arithmetic import matmul
 
         return matmul(other, self)
 
@@ -1731,7 +1770,7 @@ def __init__(self, indices, shape, dtype=np.double):
         )
 
     @property
-    def indices(self):
+    def indices(self) -> Tuple[int]:
         """Indices which will be selected when applying the linear operator to a
         vector."""
         return self._indices