Skip to content

add json serialization utils#4441

Merged
efiop merged 2 commits into
treeverse:masterfrom
skshetry:json-serialization-utils
Aug 21, 2020
Merged

add json serialization utils#4441
efiop merged 2 commits into
treeverse:masterfrom
skshetry:json-serialization-utils

Conversation

@skshetry
Copy link
Copy Markdown
Collaborator

@skshetry skshetry commented Aug 21, 2020

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

See #4415 (comment)

@skshetry skshetry self-assigned this Aug 21, 2020
@skshetry skshetry requested review from efiop, pared and pmrowla August 21, 2020 05:29
Comment thread dvc/utils/serialize/_json.py Outdated
Copy link
Copy Markdown
Contributor

@pared pared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create issue to consider moving to ujson?

@efiop efiop merged commit adce620 into treeverse:master Aug 21, 2020
@skshetry skshetry deleted the json-serialization-utils branch August 21, 2020 15:11
@skshetry
Copy link
Copy Markdown
Collaborator Author

skshetry commented Aug 27, 2020

@pared, I decided not to move to ujson as it seems, it does not support everything that the standard library supports: ultrajson/ultrajson#252.

The next target to look to is orjson which uses one of Rust's JSON library, but any new library might create difficulties due to their different levels of support and behavior, so unless there's a very strong reason, I'll choose not to.
We optimized this by ~75 fold just this week anyway. 🤷 😄

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.

4 participants