Skip to content

Commit

Permalink
Remove change_significance (#1737)
Browse files Browse the repository at this point in the history
<!--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:
- [ ] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #xyz
- [ ] Tests for the changes have been added (for bug fixes / features)
- [x] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [ ] CHANGES.rst has been updated (with summary of main changes)
- [ ] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added

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

* `change_significance` was marked as deprecated in 0.47, to be removed
in 0.49, which is the next version.

### Does this PR introduce a breaking change?
Technically, no.


### Other information:
  • Loading branch information
aulemahal authored Apr 29, 2024
2 parents 070e61b + fd05912 commit de7ce84
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 70 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ New indicators
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`).
* The previously deprecated function ``xclim.ensembles.change_significance`` has been removed. (:pull:`1737`).
* Indicators ``snw_season_length`` and ``snd_season_length`` have been modified, see above.
* The `hargeaves85`/`hg85` method for the ``potential_evapotranspiration`` indicator and indice has been modified for precision and consistency with recent academic literature. (:issue:`1710`, :pull:`1723`).


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`).
Expand Down
3 changes: 0 additions & 3 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ Ensembles Module
.. autofunction:: xclim.ensembles.robustness_categories
:noindex:

.. autofunction:: xclim.ensembles.change_significance
:noindex:

.. autofunction:: xclim.ensembles.robustness_coefficient
:noindex:

Expand Down
1 change: 0 additions & 1 deletion xclim/ensembles/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
plot_rsqprofile,
)
from ._robustness import (
change_significance,
robustness_categories,
robustness_coefficient,
robustness_fractions,
Expand Down
69 changes: 3 additions & 66 deletions xclim/ensembles/_robustness.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

from __future__ import annotations

import warnings
from inspect import Parameter, signature

import numpy as np
Expand All @@ -20,7 +19,6 @@
from xclim.indices.generic import compare, detrend

__all__ = [
"change_significance",
"robustness_categories",
"robustness_coefficient",
"robustness_fractions",
Expand All @@ -33,7 +31,7 @@
New tests must be decorated with :py:func:`significance_test` and fulfill the following requirements:
- Function name should begin by "_", registered test name is the function name without its first character and with _ replaced by -.
- Function must accept 2 positional arguments : fut and ref (see :py:func:`change_significance` for definitions)
- Function must accept 2 positional arguments : fut and ref (see :py:func:`robustness_fractions` for definitions)
- Function may accept other keyword-only arguments.
- Function must return 2 values :
+ `changed` : 1D boolean array along `realization`. True for realization with significant change.
Expand All @@ -42,7 +40,7 @@


def significance_test(func):
"""Register a significance test for use in :py:func:`change_significance`.
"""Register a significance test for use in :py:func:`robustness_fractions`.
See :py:data:`SIGNIFICANCE_TESTS`.
"""
Expand Down Expand Up @@ -294,67 +292,6 @@ def robustness_fractions( # noqa: C901
return out


def change_significance( # noqa: C901
fut: xr.DataArray | xr.Dataset,
ref: xr.DataArray | xr.Dataset,
test: str | None = "ttest",
weights: xr.DataArray | None = None,
p_vals: bool = False,
**kwargs,
) -> (
tuple[xr.DataArray | xr.Dataset, xr.DataArray | xr.Dataset]
| tuple[
xr.DataArray | xr.Dataset,
xr.DataArray | xr.Dataset,
xr.DataArray | xr.Dataset | None,
]
):
"""Backwards-compatible implementation of :py:func:`robustness_fractions`."""
warnings.warn(
(
"Function change_significance is deprecated as of xclim 0.47 and will be removed in 0.49. "
"Please use robustness_fractions instead."
),
FutureWarning,
)

if isinstance(fut, xr.Dataset):
outs = {
v: robustness_fractions(
fut[v],
ref[v] if isinstance(ref, xr.Dataset) else ref,
test=test,
weights=weights[v] if isinstance(weights, xr.Dataset) else weights,
**kwargs,
)
for v in fut.data_vars.keys()
}
change_frac = xr.merge([fracs.changed.rename(v) for v, fracs in outs.items()])
pos_frac = xr.merge(
[
(fracs.changed_positive / fracs.changed).rename(v)
for v, fracs in outs.items()
]
)
if p_vals:
if "pvals" in list(outs.values())[0]:
pvals = xr.merge([fracs.pvals.rename(v) for v, fracs in outs.items()])
else:
pvals = None
return change_frac, pos_frac, pvals
return change_frac, pos_frac

fracs = robustness_fractions(fut, ref, test=test, weights=weights, **kwargs)
# different def.
# Old "pos_frac" is fraction of change_frac that is positive
# New change_pos_frac is fraction of all that is both positive and significant
pos_frac = fracs.changed_positive / fracs.changed

if p_vals:
return fracs.changed, pos_frac, fracs.pvals if "pvals" in fracs else None
return fracs.changed, pos_frac


def robustness_categories(
changed_or_fractions: xr.Dataset | xr.DataArray,
agree: xr.DataArray | None = None,
Expand Down Expand Up @@ -667,7 +604,7 @@ def _ipcc_ar6_c(fut, ref, *, ref_pi=None):
return changed, None


# Add doc of each significance test to the `change_significance` output.
# Add doc of each significance test to `robustness_fractions` output's doc.
def _gen_test_entry(namefunc):
name, func = namefunc
doc = func.__doc__.replace("\n ", "\n\t\t").rstrip()
Expand Down

0 comments on commit de7ce84

Please sign in to comment.