Add removeAbovePercentile and removeBelowPercentile functions #992
Conversation
docs/graphite.md
Outdated
| | scale(seriesList, num) series | | Stable | | ||
| | removeAbovePercentile(seriesList, n) seriesList | | Stable | | ||
| | removeBelowPercentile(seriesList, n) seriesList | | Stable | | ||
| | scale(seriesLists, num) series | | Stable | |
There was a problem hiding this comment.
why the change to scale? graphite's scale only takes a single serieslist. do we allow multiple? (it's possible, would need to dig into the code)
There was a problem hiding this comment.
Sorry this is merge weirdness. It used to say seriesLists incorrectly, and I fixed it in one of my commits, but when I rebased this branch, it pulled the old value.
There was a problem hiding this comment.
ok. anyway you should rebase again in master to use the new table format. at least it should cause less conflicts now.
|
|
||
| if s.n <= 0 { | ||
| return nil, errors.New("The requested percent is required to be greater than 0") | ||
| } |
There was a problem hiding this comment.
should be a validator on the arg as returned by signature. that way we can check it in advance, instead of waiting until we've done a bunch of other work like fetching the data.
| return output, nil | ||
| } | ||
|
|
||
| func getPercentileValue(datapoints []schema.Point, n float64, sortedDatapointVals []float64) float64 { |
There was a problem hiding this comment.
this function should be documented better. what is sortedDatapointVals ? looks a slice for this function to use as it pleases. also what's the expected/valid range for n? is it 0 < n <= 100 ?
| } | ||
|
|
||
| var output []models.Series | ||
| sortedDatapointVals := make([]float64, 0, len(series[0].Datapoints)) //reuse float64 slice |
There was a problem hiding this comment.
confusing comment. maybe "will be reused for each getPercentileValue call" ?
stivenbb
force-pushed
the
removeAboveBelowPercentile
branch
from
08b8c11 to
ddc1e0f
Compare
|
rebased on top of master |
|
|
||
| sort.Float64s(sortedDatapointVals) | ||
|
|
||
| index := math.Min(math.Ceil(n/100.0*float64(len(sortedDatapointVals)+1)), float64(len(sortedDatapointVals))) - 1 |
There was a problem hiding this comment.
does this correspond to the method used by graphite? it looks different.
There was a problem hiding this comment.
Yes, I just simplified it.
Native implementation of removeAbovePercentile() and removeBelowPercentile()](http://graphite.readthedocs.io/en/latest/functions.html#graphite.render.functions.removeBelowPercentile) Graphite functions.