colearendt / tidyjson

Tidy your JSON data in R with tidyjson
Other
182 stars 15 forks source link

Change j* functions to be consistent with purrr #93

Open jeremystan opened 7 years ago

jeremystan commented 7 years ago

From discussion with Hadley:

For programming, spread_values(x = jstring(x)) is better, as it guarantees you will get a column "x". I am wondering if I should change jstring to json_chr, jnumber to json_dbl, jlogical to jsonlgl to mirror their map* equivalents. https://github.com/jeremystan/tidyjson/blob/master/R/spread_values.R#L122

Yeah, I think that would be a good idea.

colearendt commented 7 years ago

Wondering whether the is_json* flavor of functions should follow suit. At least for number, string, logical, I think that makes sense. Not sure how to handle null, object, array, scalar, since I expect they do not have purrr equivalents.

colearendt commented 7 years ago

This affects the append_value_* family of functions as well. I changed to append_chr,append_dbl, and append_lgl, respectively.

Not sure if this conflicts with the consistency of json_types outputting "string","number","logical" as types.

Another thing I like the idea of supporting (since JSON spec supports this) is "integer" types. At present, since json_types does not support integers, this seems hard to support for append_int, but json_int might be feasible.

colearendt commented 7 years ago

Now that I have implemented this on my branch, I'm not so sure it is right. See naming conventions on readr, which are much closer to tidyjsons original naming. Further, I think the approach readr takes is probably closer to what tidyjson ought to do - perhaps incorporating some of the parsing behavior would make spread_values and spread_all more similar in behavior... or even consolidate them.

This is relevant for discussion in #101 as well