Fix time indexer encoder and parser in fitting standardized index and… (

#1843)

<!--Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #1842 
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGELOG.rst has been updated (with summary of main changes)
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added

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

Fix the encoder and parser for the time indexer in the
`xc.indices.stats.standardized_index_fit_params` and
`xc.indices.stats.standardized_index` to allow no indexer, non-array
indexers, and multiple indexers.

### Does this PR introduce a breaking change?

No. But the new parser is not compatible with the previous as the format
of params.attrs["time_indexer"] is different. If any saved `params` from
previous version were reloaded, it may lead to errors.

### Other information:

.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": [

AUTHORS.rst

@@ -44,3 +44,4 @@ Contributors
 * Dante Castro <[email protected]> `@profesorpaiche <https://github.com/profesorpaiche>`_
 * Sascha Hofmann <[email protected]> `@saschahofmann <https://github.com/saschahofmann>`_
 * Javier Diez-Sierra <[email protected]> `@JavierDiezSierra <https://github.com/JavierDiezSierra>`_
+* Hui-Min Wang `@Hem-W <https://github.com/Hem-W>`

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
 ^^^^^^^^^^^^^^^^

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)
 

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."