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

Moves to canonical extended JSON output #147

Closed
wants to merge 1 commit into from

Conversation

atheriel
Copy link
Contributor

@atheriel atheriel commented Aug 1, 2018

As discussed in jeroen/jsonlite#247, mongolite is currently producing so-called "legacy" extended JSON in some places. This commit moves existing uses of bson_as_json() to bson_as_canonical_extended_json() in an effort to resolve this.

@jeroen
Copy link
Owner

jeroen commented Aug 2, 2018

Hmm I really don't like that the new spec wraps every integer and number is now wrapped in another object. This creates a lot of overhead, and in R we rarely distinguish these types.

   "Int32": {
       "$numberInt": "42"
   },
   "Int64": {
       "$numberLong": "42"
   },
   "Double": {
       "$numberDouble": "42.42"
   },

If we switch I think I would prefer the 'relaxed' extended json.

@atheriel
Copy link
Contributor Author

atheriel commented Aug 2, 2018

Do you mean in terms of the R objects generated? If so, I definitely favour efficiency. Otherwise I think this change only applies to generating JSON strings -- and there it might make more sense to favour correctness.

@jeroen
Copy link
Owner

jeroen commented Aug 3, 2018

I really don't like how inserting some data from R and then exporting it blows up the json because each value is wrapped in "$numberDouble": "42.42". This blows up the format and kills the interoperability, which defeats the purpose of JSON.

Generally I don't think R users care about this. In R all numbers get silently coerced to doubles. Hence I prefer the "relaxed" format which I think is more in line with the R philosophy.

@atheriel
Copy link
Contributor Author

atheriel commented Aug 4, 2018

Makes sense to me. 👍

@atheriel atheriel deleted the use-canonical-json branch August 4, 2018 21:37
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