toJSON drops names of named vectors
wch opened this issue · 17 comments
For example:
jsonlite::toJSON(list(c(a="A", b="B")))
# [["A","B"]]
This differs from the behavior of RJSONIO:
cat(RJSONIO::toJSON(list(c(a="A", b="B"))))
# [
# {
# "a": "A",
# "b": "B"
# }
It would be great to have an option to control the behavior of this. (There are some features of Shiny don't work correctly because they send named vectors and expect them to be translated into objects instead of arrays).
I'm not so sure if this is a good idea. We could add it as some sort of legacy mode, but I really believe you are better off without this feature. Here my concern:
- Type safety: jsonlite tries to consistently map every R class to a particular json structure, so that the consumer of the json knows what to expect even if the data is dynamic and the content is unknown. In R, adding a names attribute to a vector is harmless, and many functions quite liberally add or drop names to vectors. However if the json output of a particular vector completely changes depending on the presence of a
names
attribute, you can get bugs where the client was expecting an array but got an object. - Using json objects exclusively for lists and data frames results in much better reversibility. If we start mapping named vectors to json objects, then people are going to expect that
fromJSON
will simplify the json objects back into vectors. This opens up a new pandora's box of edge cases. - It is very easy for the user to use
as.list
on their named vector and get the desired json output. - R does not allow for
x$foo
for named vectors anymore (it used to), so it is not quite the same as a dictionary. IMO, the names attribute is commonly used for annotation rather than as a primary key (even though R does allow forx["foo"]
) - Atomic vectors are the building blocks for matrices and data frames. We will have to come up with rules for how to map named vectors when they appear in here. RJSONIO has not done this and is broken for these cases.
- If we are going to incorporate the
names
attribute for vectors, we are probably going to need to incorporates row and column names for matrices as well, which makes things even more complex and will completely destroy the reversibility of the json mapping.
So all in all I feel it is more safe and simple to require users to as.list
their named vector if they want it to turn it into a json object, rather than add more magic to the json encoder. Of course I understand the argument of keeping things as much as possible compatible with RJSONIO to easy the transition for shiny users, but I think in the long run adding this option will cause you more damage than it solves.
Those are all fair points. However... there is already quite a bit of code out there for Shiny, and I know that some of it uses named vectors with the expectation that they will be turned into JSON objects. This code isn't common, but it is out there.
How about having a backward-compatibility option for this, where it will output JSON objects and also print a message saying that a named list should be used instead? Then I think after some time, it could be changed to an error, and then later it could be removed.
Ugh, first thing I try:
x <- structure(as.complex(1:3), names=c("foo", "bar", "baz"))
RJSONIO::toJSON(x)
Another reason that I'd like to move away from RJSONIO. :)
This is also interesting:
x <- matrix(1:4, 2)
names(x) <- letters[1:4]
cat(RJSONIO::toJSON(x))
It gets even better when you add:
colnames(x) <- c("foo", "bar")
cat(RJSONIO::toJSON(x))
I'm not sure if there is a sensible way to add compatibility with an implementation that is broken for so many edge cases :/
Well, I was just planning on making it work for 1-d named vectors, and ignoring matrices and arrays with names. As far as I'm concerned, those can be treated as though they're unnamed, and I don't feel a need to keep backward compatibility with them.
I think that in order to migrate shiny to jsonlite, we need at least some way of working with named 1-d vectors, otherwise people will start experience mysterious breakage (that's how I encountered this issue in the first place). My thought is that the compatibility option would just be there as a stopgap solution, until developers have enough time to switch their code to named lists.
I've written a partial implementation for asJSON.numeric
, which has a part that looks like this (it may need modification to work with arrays and matrices):
if (isTRUE(keep_vec_names) && !is.null(names(x))) {
message("Input to asJSON(keep_vec_names=TRUE) is a named vector. ",
"In a future version of jsonlite, this option will not be supported, ",
"and named vectors will be translated into arrays instead of objects. ",
"If you need JSON object output, please use a named list. See ?toJSON.")
return(asJSON(as.list(x), digits = digits, use_signif = use_signif, na = na,
auto_unbox = TRUE, collapse = collapse, ...))
}
This is similar to the RJSONIO implementation. It will result in strange output if the vector at hand is part of a data frame or matrix.
Oh, good point. I'll look into this some more later and hopefully I'll come up with something that's not too horrible.
We could also not change the mapping, but still give a warning/error for named vectors. Then the shiny apps will still be broken, but it will be immediately obvious what the problem is and we can help people fix it.
I'm really reluctant to just allow apps to break. It's really unpleasant for a user to upgrade their packages and have it break their code. It's even worse if a package has a Shiny app that breaks - then the package has to be updated, submitted, accepted, and the user has to install the updated package.
I think that perfect compatibility with RJSONIO isn't necessary. For example, as you pointed out, RJSONIO's conversion of named matrices is almost nonsensical, and I doubt anyone would want that output -- but I believe that it's not that rare for users to expect named vectors to turn into JSON objects.
So I am getting the warning
Input to asJSON(keep_vec_names=TRUE) is a named vector. In a future version of jsonlite, this option will not be supported, and named vectors will be translated into arrays instead of objects. If you want JSON object output, please use a named list instead. See ?toJSON.
in a (rather large) shiny app. How do I find out which part of my app is causing this?
If you use options(warn=2)
, it will raise the warning to an error, and then you'll get a stack trace. If you use the latest version of Shiny from CRAN, it has better stack trace printing, so that should help you find the source of the problem.
Thanks for the lightning fast reply! I am using the latest shiny 0.13.1. However, setting options(warn = 2)
does not give me a stack trace. I tried also setting options(shiny.error = recover)
and options(error = recover)
. The latter finally produced a stack trace but only after I manually stop the app. Is this expected?
Anyway, the stack trace I see does not involve any of my app code but to me looks like shiny internals...
Now, I've turned on options(shiny.trace = TRUE)
, and I see a somewhat frightening Warning: Error in : C stack usage 14770274 is too close to the limit
right after the asJSON(keep_vec_names=TRUE)
warnings. Guess I'll have to read up on the debugging techniques you guys describe at http://shiny.rstudio.com/articles/debugging.html
Hi @mattflor , I am having this issue as well. Have you found a way to get a stack trace?
Well, as described in the debugging article there's options(shiny.trace = TRUE)
as well as options(shiny.fullstacktrace = TRUE)
. If I remember correctly, it turned out that in my case I was inadvertently sending a large object to a hidden output slot which resulted in both the asJSON
warnings and the C stack error.
Thank you!