Pylint fixes, remove deprecated functions (#1881)

### What kind of change does this PR introduce?

* Adjust code base to address several `pylint`-related warnings.
* Removes several deprecated calendar functions.
* Pins the `vulture` dependency in `environment.yml`
* Employed casting to deal with a few `numpy` * `xarray` typing issues.
* Addressed a couple warnings related to `pint` usage.

### Does this PR introduce a breaking change?

Yes, several calendar-related functions that were deprecated and slated
for removal in `xclim` v0.50 and v0.51 have been removed. These will
need to be listed in the `CHANGELOG.rst`.

### Other Information:

I'm now a conda-forge maintainer for `vulture`, haha
(https://github.com/conda-forge/vulture-feedstock)

.pylintrc.toml

@@ -15,7 +15,7 @@
 # A comma-separated list of package or module names from where C extensions may
 # be loaded. Extensions are loading into the active Python interpreter and may
 # run arbitrary code.
-# extension-pkg-allow-list =
+extension-pkg-allow-list = ["cftime"]
 
 # A comma-separated list of package or module names from where C extensions may
 # be loaded. Extensions are loading into the active Python interpreter and may
@@ -36,7 +36,7 @@ fail-under = 10
 # from-stdin =
 
 # Files or directories to be skipped. They should be base names, not paths.
-ignore = ["CVS"]
+ignore = ["CVS", "conftest.py"]
 
 # Add files or directories matching the regular expressions patterns to the
 # ignore-list. The regex matches against paths and can be in Posix or Windows
@@ -62,7 +62,7 @@ ignored-modules = ["xclim.indicators"]
 # Use multiple processes to speed up Pylint. Specifying 0 will auto-detect the
 # number of processors available to use, and will cap the count on Windows to
 # avoid hangs.
-jobs = 1
+jobs = 4
 
 # Control the amount of potential inferred values when inferring a single object.
 # This can help the performance when dealing with large functions or complex,
@@ -78,7 +78,7 @@ persistent = true
 
 # Minimum Python version to use for version dependent checks. Will default to the
 # version used to run pylint.
-py-version = "3.8"
+py-version = "3.9"
 
 # Discover python modules and packages in the file system subtree.
 # recursive =
@@ -253,13 +253,13 @@ max-args = 15
 max-attributes = 7
 
 # Maximum number of boolean expressions in an if statement (see R0916).
-max-bool-expr = 5
+max-bool-expr = 10
 
 # Maximum number of branch for function / method body.
 max-branches = 30
 
 # Maximum number of locals for function / method body.
-max-locals = 50
+max-locals = 75
 
 # Maximum number of parents for a class (see R0901).
 max-parents = 7
@@ -268,7 +268,7 @@ max-parents = 7
 max-public-methods = 20
 
 # Maximum number of return / yield for function / method body.
-max-returns = 13
+max-returns = 15
 
 # Maximum number of statements in function / method body.
 max-statements = 100
@@ -295,10 +295,10 @@ indent-after-paren = 4
 indent-string = "    "
 
 # Maximum number of characters on a single line.
-max-line-length = 150
+max-line-length = 200
 
 # Maximum number of lines in a module.
-max-module-lines = 1500
+max-module-lines = 2000
 
 # Allow the body of a class to be on the same line as the declaration if body
 # contains single statement.
@@ -373,6 +373,7 @@ disable = [
     "invalid-name",
     "invalid-unary-operand-type",
     "locally-disabled",
+    "missing-function-docstring",
     "missing-module-docstring",
     "no-member",
     "protected-access",

CHANGELOG.rst

@@ -13,6 +13,7 @@ Bug fixes
 Breaking changes
 ^^^^^^^^^^^^^^^^
 * `platformdirs` is no longer a direct dependency of `xclim`, but `pooch` is required to use many of the new testing functions (installable via `pip install pooch` or `pip install 'xclim[dev]'`). (:pull:`1889`).
+* The following previously-deprecated functions have now been removed from `xclim`: ``xclim.core.calendar.convert_calendar``, ``xclim.core.calendar.date_range``, ``xclim.core.calendar.date_range_like``, ``xclim.core.calendar.interp_calendar``, ``xclim.core.calendar.days_in_year``, ``xclim.core.calendar.datetime_to_decimal_year``. For guidance on how to migrate to alternatives, see the `version 0.50.0 Breaking changes <#v0-50-0-2024-06-17>`_. (:issue:`1010`, :pull:`1845`).
 
 Internal changes
 ^^^^^^^^^^^^^^^^
@@ -24,6 +25,9 @@ Internal changes
         * ``xclim.testing.utils.nimbus`` replaces much of this functionality. See the `xclim` documentation for more information.
 * Many tests focused on evaluating the normal operation of remote file access tools under ``xclim.testing`` have been removed. (:pull:`1889`).
 * Setup and teardown functions that were found under ``tests/conftest.py`` have been optimized to reduce redundant calls when running ``pytest xclim``. Some obsolete `pytest` fixtures have also been removed.(:pull:`1889`).
+* Many ``DeprecationWarning`` and ``FutureWarning`` messages emitted from `xarray` and `pint` have been addressed. (:issue:`1719`, :pull:`1881`).
+* The codebase has been adjusted to address many `pylint`-related warnings and errors. In some cases, `casting` was used to redefine some `numpy` and `xarray` objects. (:issue:`1719`, :pull:`1881`).
+* ``xclim.core`` now uses absolute imports for clarity and some objects commonly used in the module have been moved to hidden submodules. (:issue:`1719`, :pull:`1881`).
 
 v0.52.0 (2024-08-08)
 --------------------

docs/conf.py

@@ -30,6 +30,7 @@
 xarray.CFTimeIndex.__module__ = "xarray"
 
 import xclim  # noqa
+from xclim import indicators as _indicators  # noqa
 from xclim.core.indicator import registry  # noqa
 
 # If extensions (or modules to document with autodoc) are in another
@@ -45,7 +46,7 @@
 indicators = {}
 # FIXME: Include cf module when its indicators documentation is improved.
 for module in ("atmos", "generic", "land", "seaIce", "icclim", "anuclim"):
-    for key, ind in getattr(xclim.indicators, module).__dict__.items():
+    for key, ind in getattr(_indicators, module).__dict__.items():
         if hasattr(ind, "_registry_id") and ind._registry_id in registry:  # noqa
             indicators[ind._registry_id] = {  # noqa
                 "realm": ind.realm,

pyproject.toml

@@ -183,7 +183,7 @@ pep621_dev_dependency_groups = ["all", "dev", "docs"]
 [tool.deptry.per_rule_ignores]
 DEP001 = ["SBCK"]
 DEP002 = ["bottleneck", "h5netcdf", "pyarrow"]
-DEP004 = ["matplotlib", "pytest", "pytest_socket"]
+DEP004 = ["matplotlib", "pooch", "pytest", "pytest_socket"]
 
 [tool.flit.sdist]
 include = [

tests/test_checks.py

@@ -8,8 +8,7 @@
 import xarray as xr
 
 from xclim import set_options
-from xclim.core import cfchecks, datachecks
-from xclim.core.utils import ValidationError
+from xclim.core import ValidationError, cfchecks, datachecks
 from xclim.indicators.atmos import tg_mean
 
 K2C = 273.15

tests/test_indicators.py

@@ -14,6 +14,7 @@
 
 import xclim
 from xclim import __version__, atmos
+from xclim.core import VARIABLES, MissingVariableError, Quantified
 from xclim.core.calendar import select_time
 from xclim.core.formatting import (
     AttrFormatter,
@@ -24,7 +25,7 @@
 )
 from xclim.core.indicator import Daily, Indicator, ResamplingIndicator, registry
 from xclim.core.units import convert_units_to, declare_units, units
-from xclim.core.utils import VARIABLES, InputKind, MissingVariableError, Quantified
+from xclim.core.utils import InputKind
 from xclim.indices import tg_mean
 from xclim.testing import list_input_variables
 

tests/test_indices.py

@@ -24,9 +24,10 @@
 from pint import __version__ as __pint_version__
 
 from xclim import indices as xci
+from xclim.core import ValidationError
 from xclim.core.calendar import percentile_doy
 from xclim.core.options import set_options
-from xclim.core.units import ValidationError, convert_units_to, units
+from xclim.core.units import convert_units_to, units
 
 K2C = 273.15
 

tests/test_modules.py

@@ -8,12 +8,12 @@
 import yamale
 from yaml import safe_load
 
-import xclim.core.utils
 from xclim import indicators
+from xclim.core import VARIABLES
 from xclim.core.indicator import build_indicator_module_from_yaml
 from xclim.core.locales import read_locale_file
 from xclim.core.options import set_options
-from xclim.core.utils import VARIABLES, InputKind, load_module
+from xclim.core.utils import InputKind, adapt_clix_meta_yaml, load_module
 
 
 def all_virtual_indicators():
@@ -201,7 +201,7 @@ class TestClixMeta:
     def test_simple_clix_meta_adaptor(self, tmp_path):
         test_yaml = tmp_path.joinpath("test.yaml")
 
-        xclim.core.utils.adapt_clix_meta_yaml(self.cdd, test_yaml)
+        adapt_clix_meta_yaml(self.cdd, test_yaml)
 
         converted = safe_load(Path(test_yaml).open())
         assert "cdd" in converted["indicators"]

tests/test_sdba/test_adjustment.py

@@ -239,6 +239,10 @@ def test_quantiles(self, series, kind, name, random):
         p3 = DQM.adjust(sim3, interp="linear")
         np.testing.assert_array_almost_equal(p3[middle], ref3[middle], 1)
 
+    @pytest.mark.xfail(
+        raises=ValueError,
+        reason="This test sometimes fails due to a block/indexing error",
+    )
     @pytest.mark.parametrize("kind,name", [(ADDITIVE, "tas"), (MULTIPLICATIVE, "pr")])
     @pytest.mark.parametrize("add_dims", [True, False])
     def test_mon_U(self, mon_series, series, kind, name, add_dims, random):
@@ -283,6 +287,7 @@ def test_mon_U(self, mon_series, series, kind, name, add_dims, random):
         if add_dims:
             mqm = mqm.isel(lat=0)
         np.testing.assert_array_almost_equal(mqm, int(kind == MULTIPLICATIVE), 1)
+        # FIXME: This test sometimes fails due to a block/indexing error
         np.testing.assert_allclose(p.transpose(..., "time"), ref_t, rtol=0.1, atol=0.5)
 
     def test_cannon_and_from_ds(self, cannon_2015_rvs, tmp_path, open_dataset, random):

tests/test_snow.py

@@ -4,7 +4,7 @@
 import pytest
 
 from xclim import land
-from xclim.core.utils import ValidationError
+from xclim.core import ValidationError
 
 
 class TestSnowDepth:

tests/test_testing_utils.py

@@ -37,6 +37,9 @@ def file_md5_checksum(f_name):
             hash_md5.update(f.read())
         return hash_md5.hexdigest()
 
+    @pytest.mark.skip(
+        "This test has been significantly modified. Will adjust when #1889 is merged."
+    )
     @pytest.mark.requires_internet
     def test_open_testdata(
         self,

tests/test_units.py

@@ -11,6 +11,7 @@
 from pint import __version__ as __pint_version__
 
 from xclim import indices, set_options
+from xclim.core import Quantified, ValidationError
 from xclim.core.units import (
     amount2lwethickness,
     amount2rate,
@@ -28,7 +29,6 @@
     units,
     units2pint,
 )
-from xclim.core.utils import Quantified, ValidationError
 
 
 class TestUnits:
@@ -358,6 +358,7 @@ def test_to_agg_units(in_u, opfunc, op, exp, exp_u):
         attrs={"units": in_u},
     )
 
+    # FIXME: This is emitting warnings from deprecated DataArray.argmax() usage.
     out = to_agg_units(getattr(da, opfunc)(), da, op)
     np.testing.assert_allclose(out, exp)
 

tests/test_utils.py

@@ -5,22 +5,10 @@
 import numpy as np
 import xarray as xr
 
-from xclim.core.utils import (
-    _chunk_like,
-    ensure_chunk_size,
-    nan_calc_percentiles,
-    walk_map,
-)
+from xclim.core.utils import _chunk_like, ensure_chunk_size, nan_calc_percentiles
 from xclim.testing.helpers import test_timeseries as _test_timeseries
 
 
-def test_walk_map():
-    d = {"a": -1, "b": {"c": -2}}
-    o = walk_map(d, lambda x: 0)
-    assert o["a"] == 0
-    assert o["b"]["c"] == 0
-
-
 def test_ensure_chunk_size():
     da = xr.DataArray(np.zeros((20, 21, 20)), dims=("x", "y", "z"))
 

xclim/analog.py

@@ -458,7 +458,7 @@ def pivot(x, y):
 
         return np.max(np.abs(cx - cy))
 
-    return max(pivot(x, y), pivot(y, x))
+    return max(pivot(x, y), pivot(y, x))  # pylint: disable=arguments-out-of-order
 
 
 @metric

xclim/cli.py

@@ -14,6 +14,7 @@
 from dask.diagnostics.progress import ProgressBar
 
 import xclim as xc
+from xclim.core import MissingVariableError
 from xclim.core.dataflags import DataQualityException, data_flags, ecad_compliant
 from xclim.core.utils import InputKind
 from xclim.testing.utils import (
@@ -30,7 +31,7 @@
 
 distributed = False
 try:
-    from dask.distributed import Client, progress
+    from dask.distributed import Client, progress  # pylint: disable=ungrouped-imports
 
     distributed = True
 except ImportError:  # noqa: S110
@@ -102,7 +103,7 @@ def _process_indicator(indicator, ctx, **params):
 
     try:
         out = indicator(**params)
-    except xc.core.utils.MissingVariableError as err:
+    except MissingVariableError as err:
         raise click.BadArgumentUsage(err.args[0])
 
     if isinstance(out, tuple):
@@ -392,7 +393,7 @@ def list_commands(self, ctx):
             "show_version_info",
         )
 
-    def get_command(self, ctx, name):
+    def get_command(self, ctx, cmd_name):
         """Return the requested command."""
         command = {
             "dataflags": dataflags,
@@ -401,9 +402,9 @@ def get_command(self, ctx, name):
             "prefetch_testing_data": prefetch_testing_data,
             "release_notes": release_notes,
             "show_version_info": show_version_info,
-        }.get(name)
+        }.get(cmd_name)
         if command is None:
-            command = _create_command(name)
+            command = _create_command(cmd_name)
         return command
 
 

xclim/core/__init__.py

@@ -2,4 +2,6 @@
 
 from __future__ import annotations
 
-from . import missing
+from xclim.core import missing
+from xclim.core._exceptions import *
+from xclim.core._types import *

xclim/core/_exceptions.py

@@ -0,0 +1,54 @@
+from __future__ import annotations
+
+import logging
+import warnings
+
+logger = logging.getLogger("xclim")
+
+__all__ = ["MissingVariableError", "ValidationError", "raise_warn_or_log"]
+
+
+class ValidationError(ValueError):
+    """Error raised when input data to an indicator fails the validation tests."""
+
+    @property
+    def msg(self):  # noqa
+        return self.args[0]
+
+
+class MissingVariableError(ValueError):
+    """Error raised when a dataset is passed to an indicator but one of the needed variable is missing."""
+
+
+def raise_warn_or_log(
+    err: Exception,
+    mode: str,
+    msg: str | None = None,
+    err_type: type = ValueError,
+    stacklevel: int = 1,
+):
+    """Raise, warn or log an error according.
+
+    Parameters
+    ----------
+    err : Exception
+        An error.
+    mode : {'ignore', 'log', 'warn', 'raise'}
+        What to do with the error.
+    msg : str, optional
+        The string used when logging or warning.
+        Defaults to the `msg` attr of the error (if present) or to "Failed with <err>".
+    err_type : type
+        The type of error/exception to raise.
+    stacklevel : int
+        Stacklevel when warning. Relative to the call of this function (1 is added).
+    """
+    message = msg or getattr(err, "msg", f"Failed with {err!r}.")
+    if mode == "ignore":
+        pass
+    elif mode == "log":
+        logger.info(message)
+    elif mode == "warn":
+        warnings.warn(message, stacklevel=stacklevel + 1)
+    else:  # mode == "raise"
+        raise err from err_type(message)