Skip to content

Commit

Permalink
Use simpler function in {first|last}_run (#1530)
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:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #1405
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGES.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?

* Use an intermediate step instead of `rle` in first/last run
computations

### Does this PR introduce a breaking change?
No

### Other information:
  • Loading branch information
coxipi authored Nov 30, 2023
2 parents a7165ab + e305e34 commit 99f63dd
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
5 changes: 3 additions & 2 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Changelog

v0.47.0 (unreleased)
--------------------
Contributors to this version: Juliette Lavoie (:user:`juliettelavoie`), Pascal Bourgault (:user:`aulemahal`), Trevor James Smith (:user:`Zeitsperre`), David Huard (:user:`huard`).
Contributors to this version: Juliette Lavoie (:user:`juliettelavoie`), Pascal Bourgault (:user:`aulemahal`), Trevor James Smith (:user:`Zeitsperre`), David Huard (:user:`huard`), Éric Dupuis (:user:`coxipi`).

Announcements
^^^^^^^^^^^^^
Expand All @@ -20,14 +20,15 @@ Bug fixes
^^^^^^^^^
* Fixed a bug with ``n_escore=-1`` in ``xclim.sdba.adjustment.NpdfTransform``. (:issue:`1515`, :pull:`1515`).
* In the documentation, fix the tooltips in the indicator search results. (:issue:`1524`, :pull:`1527`).
* If chunked inputs are passed to indicators ``mean_radiant_temperature`` and ``potential_evapotranspiration``, sub-calculations of the solar angle will also use the same chunks, instead of a single one of the same size as the data. (:issue:`1536`, :pull:`1542`).
* If chunked inputs are passed to indicators ``mean_radiant_temperature`` and ``potential_evapotranspiration``, sub-calculations of the solar angle will also use the same chunks, instead of a single one of the same size as the data. (:issue:`1536`, :pull:`1542`).

Internal changes
^^^^^^^^^^^^^^^^
* Pinned `cf-xarray` below v0.8.5 in Python3.8 installation to further extend legacy support. (:pull:`1519`).
* `pip check` in conda builds in GitHub workflows have been temporarily set to always pass. (:pull:`1531`).
* Configure RtD search rankings to emphasize notebooks and indicators over indices and raw source code. (:pull:`1526`).
* Addressed around 100 very basic `mypy` typing errors and call signature errors. (:pull:`1532`).
* Use the intermediate step ``_cumsum_reset_on_zero`` instead of ``rle`` which is sufficient in ``_boundary_run``. (:issue:`1405`, :pull:`1530`).

v0.46.0 (2023-10-24)
--------------------
Expand Down
7 changes: 4 additions & 3 deletions xclim/indices/run_length.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ def _boundary_run(
ufunc_1dim: str | bool,
position: str,
) -> xr.DataArray:
"""Return the index of the first item of the first run of at least a given length.
"""Return the index of the first item of the first or last run of at least a given length.
Parameters
----------
Expand Down Expand Up @@ -493,9 +493,10 @@ def find_boundary_run(runs, position):
out = coord_transform(out, da)

else:
# for "first" run, return "first" element in the run (and conversely for "last" run)
d = rle(da, dim=dim, index=position)
# _cusum_reset_on_zero() is an intermediate step in rle, which is sufficient here
d = _cumsum_reset_on_zero(da, dim=dim, index=position)
d = xr.where(d >= window, 1, 0)
# for "first" run, return "first" element in the run (and conversely for "last" run)
if freq is not None:
out = d.resample({dim: freq}).map(find_boundary_run, position=position)
else:
Expand Down

0 comments on commit 99f63dd

Please sign in to comment.