mjskay / tidybayes

Bayesian analysis + tidy data + geoms (R package)
http://mjskay.github.io/tidybayes
GNU General Public License v3.0
710 stars 59 forks source link

empty space in dimnames breaks spread_draws/gather_draws #279

Open Schmidtpk opened 3 years ago

Schmidtpk commented 3 years ago

If dimnames in an mcmc.list have empty spaces, the code breaks with

"Error: Each row of output must be identified by a unique combination of keys. [...]"

This happens for example when using nimble, which has dimnames

varname[1, 1]

instead of

varname[1,1]`

Of course, users can just manipulate the names before using tidybayes with

for(i in 1:length(mcmc.list))
  dimnames(mcmc.list[[i]])[[2]]<-gsub(' ','',dimnames(mcmc.list[[i]])[[2]])

but a change in tidybayes would make it much easier.

mjskay commented 3 years ago

You should be able to handle this using the sep argument to spread_draws() / gather_draws(). Something like spread_draws(..., sep = ", ") should do it, or if the presence of a space is inconsistent (if you sometimes have none or sometimes have more than one) you could do something like spread_draws(..., sep = ", *")

It does raise the question of whether the default value of sep should be changed. The current default is "[, ]", i.e., one comma or one space. Could change to something like "[, ] *", i.e. one comma or space optionally followed by any number of spaces. I doubt that would break anyone's existing code.

Schmidtpk commented 3 years ago

Something like spread_draws(..., sep = ", ")

Thanks. This works fine; same for the general version.

Maybe an alternative would be to throw a suggestive error or warning message in case the default change is deemed to risky. At least for me it took some time to figure out that the difference in dimnames was what caused the problem.

Also, nimble could possibly just adhere to the standards. I'll post an issue there, too.

mjskay commented 3 years ago

Good point that the error when a spec breaks could suggest use of sep.

TODOs for my benefit: