Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor global pylint checks #788

Merged
10 changes: 0 additions & 10 deletions .github/workflows/linting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
48 changes: 0 additions & 48 deletions .github/workflows/pylint_check.py

This file was deleted.

2 changes: 1 addition & 1 deletion linting-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Dependencies for code linting

pylint == 2.9.*
pylint~=2.16.2
# mypy
19 changes: 14 additions & 5 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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 #
Expand Down
22 changes: 18 additions & 4 deletions src/probnum/_config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"""ProbNum library configuration"""

import contextlib
import dataclasses
from typing import Any
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()
2 changes: 1 addition & 1 deletion src/probnum/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""

Expand Down
10 changes: 4 additions & 6 deletions src/probnum/functions/_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down
16 changes: 8 additions & 8 deletions src/probnum/linops/_arithmetic_fallbacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, ...]:
Expand Down Expand Up @@ -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)
Expand Down
57 changes: 48 additions & 9 deletions src/probnum/linops/_linear_operator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand Down
Loading