fix & improve grouped spi #50
Conversation
valpesendorfer
force-pushed
the
patch
branch
3 times, most recently
from
2c59561 to
ac470b7
Compare
| if not groups.dtype.name == "int16": | ||
| warn("Casting groups to int16!") | ||
| groups = groups.astype("int16") | ||
| groups, keys = to_linspace(np.array(groups, dtype="str")) |
There was a problem hiding this comment.
@Kirill888 the idea is that the user passes in a list / array / tuple / ... containing group labels which puts the time dimension into bins. The downstream function requires these groups to be in a linear space, so from 0 to len(groups)-1 which was the expected input previously.
To take this burden off the user, this allows any labels and converts them to a str array, which is then converted to linear space. Maybe not most efficient, but for these applications totally fine - does that make sense for you?
For a practical example, we'll be using this to calculate SPIs for the full timeseries, grouping the time dimension into dekads per year. The user can then pass simply xx.time.dekad.yidx in without having to make sure the groups array is 0 indexed.
hdc/algo/accessors.py
Outdated
|
|
||
| if calstart_ix >= calstop_ix: | ||
| raise ValueError("calibration_start < calibration_stop!") | ||
| if calibration_start > tix[-1:]: |
There was a problem hiding this comment.
is this comparing str > [str]?
There was a problem hiding this comment.
comparing str to pd.Timestamp
There was a problem hiding this comment.
why tix[-1:] and not tix[-1]?
There was a problem hiding this comment.
needs to remain a pd.DateTimeIndex with one element to work ... maybe bit hacky & counterintuitive
There was a problem hiding this comment.
we could explicitly convert the string to a date object (pd.Timestamp or whatever) and then compare the elements if that's clearer
This PR contains fixes & improvements to the code relevant for grouped (along time dim) SPIs.
Changes:
hdc.zonal.meanhdc.algo.spi