Skip to content

Conversation

@daxbert
Copy link

@daxbert daxbert commented Jul 18, 2016

Here are the changes to support saving state in TDigest.

I'm not very familiar with javascript unit testing. Would appreciate any guidance you may have.

Dax Eckenberg and others added 2 commits July 18, 2016 15:45
* added _traverse private method
    walks the binary tree calling _insert_centroid on each data node

* added _insert_centroid private method
    simply inserts an existing object into the tree

* added public load method
    takes an object and re-initializes state from that object
Issue-welch#4: Add support for saving state
@daxbert
Copy link
Author

daxbert commented Jul 18, 2016

note: I didn't bump the version as I wasn't sure what you'd prefer.

@welch
Copy link
Owner

welch commented Jul 19, 2016

Thanks for the PR!

I'm going to be swamped for a few days before I'll get
a close look at it. But it seems more complicated than
need be. I believe you can capture the needed state by
just dumping the centroids using toArray, and restore it
by calling push_centroid on each item in order (no compression
will be triggered). Let me know what you think.

@daxbert
Copy link
Author

daxbert commented Jul 19, 2016

Ah, I'll give that a try. If it gives the same results I'll cancel the
PR.

On Jul 18, 2016 11:51 PM, "Will Welch" notifications@github.com wrote:

Thanks for the PR!

I'm going to be swamped for a few days before I'll get
a close look at it. But it seems more complicated than
need be. I believe you can capture the needed state by
just dumping the centroids using toArray, and restore it
by calling push_centroid on each item in order (no compression
will be triggered). Let me know what you think.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#5 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGaSPFAe3zRn6guGVrYFyJEEonU4kyLwks5qXHQAgaJpZM4JPOGF
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants