-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
Upgrading to rs-0.46 #309
base: main
Are you sure you want to change the base?
Upgrading to rs-0.46 #309
Conversation
toJSON() { | ||
return _df.toJson().toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we don't support serialization anymore, I'd rather remove this method, along with the other serialization based ones toObject
,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do support serialization using Ipc format.
Since this is JS, I figured we need Json support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at a minimum, The toJSON
/ JSON.stringify
should use a columnar representation, not the row oriented. It also needs to work with JSON.stringify
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toJSON
and JSON.stringify(df)
produce valid JSON: "[{\"foo\":1.0,\"bar\":\"a\"},{\"foo\":2.0,\"bar\":\"b\"},{\"foo\":3.0,\"bar\":\"c\"}]"
which matches df.writeJSON(..)
What would a valid JSON column representation look like?
Upgrading:
Serialization has been changed in core RS by this PR
Adding new to_json func due to the serialization change in core RS.