Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue-#4: Add Support for saving state #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

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.

None yet

2 participants