Add aggregation type support #4
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added "../lib" path to requiring buffer_ieee754, was breaking otherwise.
…ete hack, but works]
…t is out of bounds" try/catch on all fs.read handles this, but does not solve it.
This reverts commit ea2181b.
…s offsets on fs.read
The XFilesFactor that was being passed to the create method was being ignored previously. Now it isn't. Signed-off-by: ciaranj <[email protected]>
Previously calls to updateMany would ultimately have their data discarded, as concat does not mutate the existing arrays but returns a new one, the python-ported algorithm doesn't quite apply. Signed-off-by: ciaranj <[email protected]>
This change may be un-acceptable :(. If Hoard.js is used within the same process as statsd (as for example a back-end) then it is possible that hoard will refuse updates from statsD's flush event as the timestamp that statsd generates will appear to be from the future. This is because Hoard.js was using: parseInt( new Date().getTime() / 1000 ) But statsd is using : Math.round( new Date().getTime() / 1000 ) To get their idea of 'now'. Most of the time this will be ok, but if a the result of the division contains a fractional part greater than or equal to .5 then statsd will round-up to the next nearest integer value but Hoard was truncating (discarding?) the fractional part, meaning it would be (as far as it was concerned) timestamps from the future when update was called sufficiently quickly after the flush. Signed-off-by: ciaranj <[email protected]>
Much like commit 0cc664 in the python whisper project, this commit introduces support for specifying the aggregation type (one of 'sum', 'max', 'min', 'last' or 'average') when propagating values into the lower precision archives. This commit also bumps the package number as it introduces a breaking change to the create interface by making the xFilesFactor and new aggregationMethod arguments optional (defaulting to 0.5 and average respectively). Following semver.org recommendations that means a bump to the middle value :) Signed-off-by: ciaranj <[email protected]>
|
Be aware of @3c8b006 that might be contentious! |
Signed-off-by: ciaranj <[email protected]>
|
3c8b006 is "contentious" but needed for compatibility. I like it. Let's get it merged! |
|
Just so you're aware, I've stopped working on this library, and moved over to https://github.com/ciaranj/ceres/tree/nodejs_port might be of use :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on top of the work done by @cwholt further fixes and an implementation of the aggregation method support, so now when aggregating to lower precision archives 'max, min, sum, last and average' can be used instead of just always averaging (the same as python's whisper)