From 8df004221f9be212324d70cec8bd21c3e7a04c43 Mon Sep 17 00:00:00 2001 From: Zeitsperre <10819524+Zeitsperre@users.noreply.github.com> Date: Thu, 18 Apr 2024 12:03:51 -0400 Subject: [PATCH 1/4] add vulture to check for dead code, update pre-commit checks --- .pre-commit-config.yaml | 10 +++++++--- pyproject.toml | 8 ++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bb45a186c..1cd37f8a9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,7 +9,7 @@ repos: args: ['--py39-plus'] exclude: 'xclim/core/indicator.py' - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.5.0 + rev: v4.6.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer @@ -41,7 +41,7 @@ repos: hooks: - id: isort - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.3.5 + rev: v0.3.7 hooks: - id: ruff - repo: https://github.com/pylint-dev/pylint @@ -55,6 +55,10 @@ repos: - id: flake8 additional_dependencies: [ 'flake8-alphabetize', 'flake8-rst-docstrings '] args: [ '--config=.flake8' ] + - repo: https://github.com/jendrikseipp/vulture + rev: 'v2.11' + hooks: + - id: vulture - repo: https://github.com/nbQA-dev/nbQA rev: 1.8.5 hooks: @@ -87,7 +91,7 @@ repos: hooks: - id: gitleaks - repo: https://github.com/python-jsonschema/check-jsonschema - rev: 0.28.1 + rev: 0.28.2 hooks: - id: check-github-workflows - id: check-readthedocs diff --git a/pyproject.toml b/pyproject.toml index 9f8ec848d..7dffbac8b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -312,3 +312,11 @@ max-doc-length = 180 [tool.ruff.lint.pydocstyle] convention = "numpy" + +[tool.vulture] +exclude = [] +ignore_decorators = ["@pytest.fixture"] +ignore_names = [] +min_confidence = 90 +paths = ["xclim", "tests"] +sort_by_size = true From 3cd5251b40449798b0c8bf4d0db5252287adf53e Mon Sep 17 00:00:00 2001 From: Zeitsperre <10819524+Zeitsperre@users.noreply.github.com> Date: Thu, 18 Apr 2024 12:08:16 -0400 Subject: [PATCH 2/4] add relevant noqa statements, modify some variables to address dead code segments, remove obsolete construct_moving_yearly_window and unpack_moving_yearly_window functions --- tests/test_cli.py | 2 +- tests/test_formatting.py | 2 +- tests/test_sdba/conftest.py | 2 +- tests/test_units.py | 4 +- xclim/core/calendar.py | 4 +- xclim/core/indicator.py | 2 +- xclim/core/options.py | 5 ++- xclim/core/units.py | 2 +- xclim/indices/fire/_cffwis.py | 2 +- xclim/sdba/__init__.py | 7 +-- xclim/sdba/processing.py | 85 ++++++++++++----------------------- 11 files changed, 44 insertions(+), 73 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 4be9198c0..966308e74 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -374,7 +374,7 @@ def test_release_notes_failure(method, error): assert error in results.output -def test_show_version_info(capsys): +def test_show_version_info(): runner = CliRunner() results = runner.invoke(cli, ["show_version_info"]) assert "INSTALLED VERSIONS" in results.output diff --git a/tests/test_formatting.py b/tests/test_formatting.py index 438547cca..edf827d85 100644 --- a/tests/test_formatting.py +++ b/tests/test_formatting.py @@ -47,7 +47,7 @@ def test_indicator_docstring(): def test_update_xclim_history(atmosds): @fmt.update_xclim_history - def func(da, arg1, arg2=None, arg3=None): + def func(da, arg1, arg2=None, arg3=None): # noqa: return da out = func(atmosds.tas, 1, arg2=[1, 2], arg3=None) diff --git a/tests/test_sdba/conftest.py b/tests/test_sdba/conftest.py index 4b519d4d6..d09bc33f9 100644 --- a/tests/test_sdba/conftest.py +++ b/tests/test_sdba/conftest.py @@ -105,7 +105,7 @@ def qds_month(): @pytest.fixture -def ref_hist_sim_tuto(socket_enabled): +def ref_hist_sim_tuto(socket_enabled): # noqa: F841 """Return ref, hist, sim time series of air temperature. socket_enabled is a fixture that enables the use of the internet to download the tutorial dataset while the diff --git a/tests/test_units.py b/tests/test_units.py index febcd27be..e94db9140 100644 --- a/tests/test_units.py +++ b/tests/test_units.py @@ -291,7 +291,9 @@ def dryness_index( def test_declare_relative_units(): - def index(data: xr.DataArray, thresh: Quantified, dthreshdt: Quantified): + def index( + data: xr.DataArray, thresh: Quantified, dthreshdt: Quantified # noqa: F841 + ): return xr.DataArray(1, attrs={"units": "rad"}) index_relative = declare_relative_units(thresh="", dthreshdt="/[time]")( diff --git a/xclim/core/calendar.py b/xclim/core/calendar.py index de66bbefd..0d82955b0 100644 --- a/xclim/core/calendar.py +++ b/xclim/core/calendar.py @@ -885,10 +885,10 @@ def is_offset_divisor(divisor: str, offset: str): return False # Reconstruct offsets anchored at the start of the period # to have comparable quantities, also get "offset" objects - mA, bA, sA, aA = parse_offset(divisor) + mA, bA, _sA, aA = parse_offset(divisor) offAs = pd.tseries.frequencies.to_offset(construct_offset(mA, bA, True, aA)) - mB, bB, sB, aB = parse_offset(offset) + mB, bB, _sB, aB = parse_offset(offset) offBs = pd.tseries.frequencies.to_offset(construct_offset(mB, bB, True, aB)) tB = pd.date_range("1970-01-01T00:00:00", freq=offBs, periods=13) diff --git a/xclim/core/indicator.py b/xclim/core/indicator.py index deb0125a7..6eacdb45c 100644 --- a/xclim/core/indicator.py +++ b/xclim/core/indicator.py @@ -510,7 +510,7 @@ def __new__(cls, **kwds): # noqa: C901 return super().__new__(new) @staticmethod - def _parse_indice(compute, passed_parameters): + def _parse_indice(compute, passed_parameters): # noqa: F841 """Parse the compute function. - Metadata is extracted from the docstring diff --git a/xclim/core/options.py b/xclim/core/options.py index 04560fa0c..e4f78a255 100644 --- a/xclim/core/options.py +++ b/xclim/core/options.py @@ -224,7 +224,8 @@ def __enter__(self): """Context management.""" return - def _update(self, kwargs): + @staticmethod + def _update(kwargs): """Update values.""" for k, v in kwargs.items(): if k in _SETTERS: @@ -232,6 +233,6 @@ def _update(self, kwargs): else: OPTIONS[k] = v - def __exit__(self, option_type, value, traceback): + def __exit__(self, option_type, value, traceback): # noqa: F841 """Context management.""" self._update(self.old) diff --git a/xclim/core/units.py b/xclim/core/units.py index 2d58a33f8..edd22e8a9 100644 --- a/xclim/core/units.py +++ b/xclim/core/units.py @@ -63,7 +63,7 @@ # g is the most versatile float format. units.default_format = "gcf" # Switch this flag back to False. Not sure what that implies, but it breaks some tests. -units.force_ndarray_like = False +units.force_ndarray_like = False # noqa: F841 # Another alias not included by cf_xarray units.define("@alias percent = pct") diff --git a/xclim/indices/fire/_cffwis.py b/xclim/indices/fire/_cffwis.py index 240314502..c0201e04a 100644 --- a/xclim/indices/fire/_cffwis.py +++ b/xclim/indices/fire/_cffwis.py @@ -892,7 +892,7 @@ def fire_weather_ufunc( # noqa: C901 ffmc0: xr.DataArray | None = None, winter_pr: xr.DataArray | None = None, season_mask: xr.DataArray | None = None, - start_dates: str | xr.DataArray | None = None, + start_dates: str | xr.DataArray | None = None, # noqa: F841 indexes: Sequence[str] | None = None, season_method: str | None = None, overwintering: bool = False, diff --git a/xclim/sdba/__init__.py b/xclim/sdba/__init__.py index 911d89c42..e0b03aa3c 100644 --- a/xclim/sdba/__init__.py +++ b/xclim/sdba/__init__.py @@ -9,12 +9,7 @@ from . import adjustment, detrending, measures, processing, properties, utils from .adjustment import * from .base import Grouper -from .processing import ( - construct_moving_yearly_window, - stack_variables, - unpack_moving_yearly_window, - unstack_variables, -) +from .processing import stack_variables, unstack_variables # TODO: ISIMIP ? Used for precip freq adjustment in biasCorrection.R # Hempel, S., Frieler, K., Warszawski, L., Schewe, J., & Piontek, F. (2013). A trend-preserving bias correction – diff --git a/xclim/sdba/processing.py b/xclim/sdba/processing.py index ddefddfb3..b4e904829 100644 --- a/xclim/sdba/processing.py +++ b/xclim/sdba/processing.py @@ -5,7 +5,6 @@ """ from __future__ import annotations -import warnings from collections.abc import Sequence import dask.array as dsk @@ -13,13 +12,7 @@ import xarray as xr from xarray.core.utils import get_temp_dimname -from xclim.core.calendar import ( - get_calendar, - max_doy, - parse_offset, - stack_periods, - unstack_periods, -) +from xclim.core.calendar import get_calendar, max_doy, parse_offset from xclim.core.formatting import update_xclim_history from xclim.core.units import convert_units_to, infer_context, units from xclim.core.utils import uses_dask @@ -29,6 +22,22 @@ from .nbutils import _escore from .utils import ADDITIVE, copy_all_attrs +__all__ = [ + "adapt_freq", + "escore", + "from_additive_space", + "jitter", + "jitter_over_thresh", + "jitter_under_thresh", + "normalize", + "reordering", + "stack_variables", + "standardize", + "to_additive_space", + "unstack_variables", + "unstandardize", +] + @update_xclim_history def adapt_freq( @@ -206,24 +215,24 @@ def jitter( minimum = convert_units_to(minimum, x) if minimum is not None else 0 minimum = minimum + np.finfo(x.dtype).eps if uses_dask(x): - jitter = dsk.random.uniform( + jitter_dist = dsk.random.uniform( low=minimum, high=lower, size=x.shape, chunks=x.chunks ) else: - jitter = np.random.uniform(low=minimum, high=lower, size=x.shape) - out = out.where(~((x < lower) & notnull), jitter.astype(x.dtype)) + jitter_dist = np.random.uniform(low=minimum, high=lower, size=x.shape) + out = out.where(~((x < lower) & notnull), jitter_dist.astype(x.dtype)) if upper is not None: if maximum is None: raise ValueError("If 'upper' is given, so must 'maximum'.") upper = convert_units_to(upper, x) maximum = convert_units_to(maximum, x) if uses_dask(x): - jitter = dsk.random.uniform( + jitter_dist = dsk.random.uniform( low=upper, high=maximum, size=x.shape, chunks=x.chunks ) else: - jitter = np.random.uniform(low=upper, high=maximum, size=x.shape) - out = out.where(~((x >= upper) & notnull), jitter.astype(x.dtype)) + jitter_dist = np.random.uniform(low=upper, high=maximum, size=x.shape) + out = out.where(~((x >= upper) & notnull), jitter_dist.astype(x.dtype)) copy_all_attrs(out, x) # copy attrs and same units return out @@ -484,42 +493,6 @@ def _get_number_of_elements_by_year(time): return int(N_in_year) -def construct_moving_yearly_window( - da: xr.Dataset, window: int = 21, step: int = 1, dim: str = "movingwin" -): - """Deprecated function. - - Use :py:func:`xclim.core.calendar.stack_periods` instead, renaming ``step`` to ``stride``. - Beware of the different default value for `dim` ("period"). - """ - warnings.warn( - FutureWarning, - ( - "`construct_moving_yearly_window` is deprecated and will be removed in a future version. " - f"Please use xclim.core.calendar.stack_periods(da, window={window}, stride={step}, dim='{dim}', freq='YS') instead." - ), - ) - return stack_periods(da, window=window, stride=step, dim=dim, freq="YS") - - -def unpack_moving_yearly_window( - da: xr.DataArray, dim: str = "movingwin", append_ends: bool = True -): - """Deprecated function. - - Use :py:func:`xclim.core.calendar.unstack_periods` instead. - Beware of the different default value for `dim` ("period"). The new function always behaves like ``appends_ends=True``. - """ - warnings.warn( - FutureWarning, - ( - "`unpack_moving_yearly_window` is deprecated and will be removed in a future version. " - f"Please use xclim.core.calendar.unstack_periods(da, dim='{dim}') instead." - ), - ) - return unstack_periods(da, dim=dim) - - @update_xclim_history def to_additive_space( data: xr.DataArray, @@ -754,17 +727,17 @@ def stack_variables(ds: xr.Dataset, rechunk: bool = True, dim: str = "multivar") # Store original arrays' attributes attrs = {} # sort to have coherent order with different datasets - datavars = sorted(ds.data_vars.items(), key=lambda e: e[0]) - nvar = len(datavars) - for i, (nm, var) in enumerate(datavars): + data_vars = sorted(ds.data_vars.items(), key=lambda e: e[0]) + nvar = len(data_vars) + for i, (nm, var) in enumerate(data_vars): for name, attr in var.attrs.items(): - attrs.setdefault("_" + name, [None] * nvar)[i] = attr + attrs.setdefault(f"_{name}", [None] * nvar)[i] = attr # Special key used for later `unstacking` attrs["is_variables"] = True - var_crd = xr.DataArray([nm for nm, vr in datavars], dims=(dim,), name=dim) + var_crd = xr.DataArray([nm for nm, vr in data_vars], dims=(dim,), name=dim) - da = xr.concat([vr for nm, vr in datavars], var_crd, combine_attrs="drop") + da = xr.concat([vr for nm, vr in data_vars], var_crd, combine_attrs="drop") if uses_dask(da) and rechunk: da = da.chunk({dim: -1}) From 77a279faee3cec9514a8e2ee8ccbe4f9d5d2a6ea Mon Sep 17 00:00:00 2001 From: Zeitsperre <10819524+Zeitsperre@users.noreply.github.com> Date: Thu, 18 Apr 2024 12:21:48 -0400 Subject: [PATCH 3/4] add vulture to tox and makefile linting recipes, add to dev requirements --- Makefile | 1 + pyproject.toml | 1 + tests/test_formatting.py | 2 +- tox.ini | 2 ++ 4 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 60b1597bc..d7f585984 100644 --- a/Makefile +++ b/Makefile @@ -57,6 +57,7 @@ lint: ## check style with flake8 and black isort --check xclim tests ruff xclim tests flake8 --config=.flake8 xclim tests + vulture xclim tests nbqa black --check docs blackdoc --check --exclude=xclim/indices/__init__.py xclim blackdoc --check docs diff --git a/pyproject.toml b/pyproject.toml index 7dffbac8b..7ad9704c3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,6 +84,7 @@ dev = [ "tox >=4.0", # "tox-conda", # Will be added when a tox@v4.0+ compatible plugin is released. "tox-gh >=1.3.1", + "vulture", "xdoctest", "yamllint", # Documentation and examples diff --git a/tests/test_formatting.py b/tests/test_formatting.py index edf827d85..56cd60efb 100644 --- a/tests/test_formatting.py +++ b/tests/test_formatting.py @@ -47,7 +47,7 @@ def test_indicator_docstring(): def test_update_xclim_history(atmosds): @fmt.update_xclim_history - def func(da, arg1, arg2=None, arg3=None): # noqa: + def func(da, arg1, arg2=None, arg3=None): # noqa: F841 return da out = func(atmosds.tas, 1, arg2=[1, 2], arg3=None) diff --git a/tox.ini b/tox.ini index afbc68cd5..cd55e0533 100644 --- a/tox.ini +++ b/tox.ini @@ -37,6 +37,7 @@ deps = isort==5.13.2 nbqa ruff>=0.1.0 + vulture yamllint commands_pre = commands = @@ -44,6 +45,7 @@ commands = isort --check xclim tests ruff xclim tests flake8 --config=.flake8 xclim tests + vulture xclim tests nbqa black --check docs blackdoc --check --exclude=xclim/indices/__init__.py xclim blackdoc --check docs From 6e44622df53509ca54129eddc3e194bc85b248bd Mon Sep 17 00:00:00 2001 From: Zeitsperre <10819524+Zeitsperre@users.noreply.github.com> Date: Thu, 18 Apr 2024 12:27:59 -0400 Subject: [PATCH 4/4] update CHANGES.rst --- CHANGES.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 5f953b2b7..d7b4480f9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,10 @@ Announcements ^^^^^^^^^^^^^ * `xclim` has migrated its development branch name from `master` to `main`. (:issue:`1667`, :pull:`1669`). +Breaking changes +^^^^^^^^^^^^^^^^ +* The previously deprecated functions ``xclim.sdba.processing.construct_moving_yearly_window`` and ``xclim.sdba.processing.unpack_moving_yearly_window`` have been removed. These functions have been replaced by ``xclim.core.calendar.stack_periods`` and ``xclim.core.calendar.unstack_periods``. (:pull:`1717`). + Bug fixes ^^^^^^^^^ * Fixed an bug in sdba's ``map_groups`` that prevented passing DataArrays with cftime coordinates if the ``sdba_encode_cf`` option was True. (:issue:`1673`, :pull:`1674`). @@ -24,6 +28,7 @@ Internal changes * Reorganized GitHub CI build matrices to run the doctests more consistently. (:pull:`1709`). * Removed the experimental `numba` and `llvm` dependency installation steps in the `tox.ini` file. Added `numba@main` to the upstream dependencies. (:pull:`1709`). * Added the `tox-gh` dependency to the development installation recipe. This will soon be required for running the `tox` test ensemble on GitHub Workflows. (:pull:`1709`). +* Added the `vulture` static code analysis tool for finding dead code to the development dependency list and linters (makefile, tox and pre-commit hooks). (:pull:`1717`). v0.48.2 (2024-02-26) --------------------