Rain season index

[coxipi]

Pull Request Checklist:

• This PR addresses an already opened issue (for bug fixes / features)
  • This PR fixes Proposal of a rain season group of climate indices #842
• Tests for the changes have been added (for bug fixes / features)
  • (If applicable) Documentation has been added / updated (for bug fixes / features)
• HISTORY.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?

• Adds a rain season index (useful notably in West Africa)

Does this PR introduce a breaking change?

No

Other information:

[coxipi]

Ok, I tried to get a closer notation to what is already used in xclim. events is a form of generalized season (without constraint on time with date).

e.g. imagine a season with window=3
[1,1,1,0,1,1,0,0,1, 0,0,0]
The season would be the 9 first entries. It begins at element 0 because of the three 1's and ends at element 9 because of the three 0's. An event==season would be:
[1,1,1,1,1,1,1,1,1, 0,0,0]
i.e. 1's in the season sequence.

With this in mind, should I change the name of rle_events? We already have season_lengths, so this could also event_lengths? Or does rle_events seem fine?

[Zeitsperre]

I'm much more a fan of function names that clearly signal what it is that they do. event_lengths is generic yet clear and concise.

[coveralls]

coverage: 90.463% (+0.05%) from 90.413% when pulling 1098846 on rain_season_index into 7dc79ba on master.

[coxipi]

I'm much more a fan of function names that clearly signal what it is that they do. event_lengths is generic yet clear and concise.

Ok sounds good. I just realize, one difference compared with season_length gives the length of the first run, just one number per time series.

Here, event_lengths would give something like:

[5,NaN,NaN,NaN,NaN,0,0,0, 3,NaN,NaN, ...]
the first event has length 5, the second event has length 3, etc.

Is this type of output ok for something we would call event_lengths? I initially called it rle_events because it is the same kind of output as rle. I see the advantage of clearer function names, I just want to make sure it's ok we call this event_lengths given the form of the output.

[Zeitsperre]

Good point. For a function called event_lengths, I'd want to be able to perform things like:

events = event_lengths(da, ...)
events.mean()
events.min()
events.mode(), etc.

But given the structure, we would be performing operations on 0 values. Hmm...

[coxipi]

Yes. In the end, rle_events is almost useless. It's just a shortcut for:

events = extract_events(...)
out = rle(events)

# same as 
out = rle_events(...)

I thought of rle_events as accomplishing a similar objective as season, but as underlined above, it's a bit different.

We could do without it (at least for now) if that simplifies things

[Zeitsperre]

There's utility in having something at that level, but it would be better to have an issue with a clear justification for it, maybe? It would also potentially affect a number of existing indexes, so knowing which ones before we start implementing it would also be good to know.

You could open an issue to discuss it further; I think we have more than covered the features we want to within the scope of this PR.

[coxipi]

Sounds good, I'm dropping it for now and referring this PR in a new issue

EDIT: Hmm the tests are explicitly for rle_events

EDIT2: I re-wrote the tests, they're better like this anyways. Waiting for green arrow

[Zeitsperre]

@coxipi @aulemahal Thanks so much for the work here!