diff --git a/.zenodo.json b/.zenodo.json index 585d12ba7..e329952de 100644 --- a/.zenodo.json +++ b/.zenodo.json @@ -139,6 +139,11 @@ "name": "Diez-Sierra, Javier", "affiliation": "Santander Meteorology Group, Instituto de FĂ­sica de Cantabria (CSIC-UC), Santander, Spain", "orcid": "0000-0001-9053-2542" + }, + { + "name": "Wang, Hui-Min", + "affiliation": "National University of Singapore, Singapore, Singapore", + "orcid": "0000-0002-5878-7542" } ], "keywords": [ diff --git a/AUTHORS.rst b/AUTHORS.rst index 0504718e7..f9a2ff924 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -44,3 +44,4 @@ Contributors * Dante Castro `@profesorpaiche `_ * Sascha Hofmann `@saschahofmann `_ * Javier Diez-Sierra `@JavierDiezSierra `_ +* Hui-Min Wang `@Hem-W ` diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 613c332c5..026917a27 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,7 +4,11 @@ Changelog v0.52.0 (unreleased) -------------------- -Contributors to this version: David Huard (:user:`huard`), Trevor James Smith (:user:`Zeitsperre`). +Contributors to this version: David Huard (:user:`huard`), Trevor James Smith (:user:`Zeitsperre`), Hui-Min Wang (:user:`Hem-W`). + +Bug fixes +^^^^^^^^^ +* Fixed the indexer bug in the `xclim.indices.standardized_index_fit_params` when multiple or non-array indexers are specified and fitted parameters are reloaded from netCDF. (:issue:`1842`, :pull:`1843`). Internal changes ^^^^^^^^^^^^^^^^ diff --git a/tests/test_indices.py b/tests/test_indices.py index 9629d22d5..702955206 100644 --- a/tests/test_indices.py +++ b/tests/test_indices.py @@ -731,7 +731,15 @@ def test_standardized_precipitation_evapotranspiration_index( np.testing.assert_allclose(spei.values, values, rtol=0, atol=diff_tol) - def test_standardized_index_modularity(self, open_dataset): + @pytest.mark.parametrize( + "indexer", + [ + ({}), + ({"month": [2, 3]}), + ({"month": [2, 3], "drop": True}), + ], + ) + def test_standardized_index_modularity(self, open_dataset, tmp_path, indexer): freq, window, dist, method = "MS", 6, "gamma", "APP" ds = ( open_dataset("sdba/CanESM2_1950-2100.nc") @@ -756,8 +764,15 @@ def test_standardized_index_modularity(self, open_dataset): dist=dist, method=method, fitkwargs=fitkwargs, - month=[2, 3], + **indexer, ) + + # Save the parameters to a file to test against that saving process may modify the netCDF file + paramsfile = tmp_path / "params0.nc" + params.to_netcdf(paramsfile) + params.close() + params = xr.open_dataset(paramsfile).__xarray_dataarray_variable__ + spei1 = xci.standardized_precipitation_evapotranspiration_index( wb.sel(time=slice("1998", "2000")), params=params ) @@ -771,13 +786,18 @@ def test_standardized_index_modularity(self, open_dataset): fitkwargs=fitkwargs, cal_start="1950", cal_end="1980", - month=[2, 3], + **indexer, ).sel(time=slice("1998", "2000")) # In the previous computation, the first {window-1} values are NaN because the rolling is performed on the period [1998,2000]. # Here, the computation is performed on the period [1950,2000], *then* subsetted to [1998,2000], so it doesn't have NaNs # for the first values - spei2[{"time": slice(0, window - 1)}] = np.nan + nan_window = xr.cftime_range( + spei1.time.values[0], periods=window - 1, freq=freq + ) + spei2.loc[{"time": spei2.time.isin(nan_window)}] = ( + np.nan + ) # select time based on the window is necessary when `drop=True` np.testing.assert_allclose(spei1.values, spei2.values, rtol=0, atol=1e-4) diff --git a/xclim/indices/stats.py b/xclim/indices/stats.py index 91eae6b14..4d3e6e167 100644 --- a/xclim/indices/stats.py +++ b/xclim/indices/stats.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import warnings from collections.abc import Sequence from typing import Any @@ -797,8 +798,7 @@ def standardized_index_fit_params( "group": group, "units": "", } - method, args = ("", []) if indexer == {} else indexer.popitem() - params.attrs["time_indexer"] = (method, *args) + params.attrs["time_indexer"] = json.dumps(indexer) if offset: params.attrs["offset"] = offset return params @@ -879,7 +879,7 @@ def standardized_index( ) # Unpack attrs to None and {} if needed freq = None if freq == "" else freq - indexer = {} if indexer[0] == "" else {indexer[0]: indexer[1:]} + indexer = json.loads(indexer) if cal_start or cal_end: warnings.warn( "Expected either `cal_{start|end}` or `params`, got both. The `params` input overrides other inputs."