Ouranosinc / xclim / 21767596042

92%

push

github

Doymin, doymax :  nan when all the same (#2314)

<!--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
- [x] Tests for the changes have been added (for bug fixes / features)
- [ ] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGELOG.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?
Rewrite of `doymin` and `doymax`. Addition of a check to put NaN when
all the values are the same.

Also, I don't know since where this is possible, but using `idxmin` is
much simpler than `argmin` with `lazy_indexing`! I think the `dt`
accessor wasn't always available with dask.

I want to remove the special treatment in snow indicators where we first
check is any value is above 0. With this new behaviour of `doymax` , the
special treatment is unnecessary for `snd_max_doy`, for example.

This PR also has the side effect of 
- allowing all-nan slices
- return a float instead of an integer

### Does this PR introduce a breaking change?
Before a slice where all values where the same would return the doy of
the first timestamp. I think this is problematic.

10 of 10 new or added lines in 1 file covered. (100.0%)

7924 of 8613 relevant lines covered (92.0%)

6.28 hits per line