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

empty space in dimnames breaks spread_draws/gather_draws #279

Open
Schmidtpk opened this issue Jan 14, 2021 · 3 comments
Open

empty space in dimnames breaks spread_draws/gather_draws #279

Schmidtpk opened this issue Jan 14, 2021 · 3 comments
Milestone

Comments

@Schmidtpk
Copy link

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
Copy link
Owner

mjskay commented Jan 15, 2021

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
Copy link
Author

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
Copy link
Owner

mjskay commented Jan 16, 2021

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

TODOs for my benefit:

  • consider changing default of sep arg to something like "[, ] *"
  • add a note in errors in spread_draws suggesting people look at sep if it seems appropriate

@mjskay mjskay added this to the Next release milestone Jun 29, 2021
@mjskay mjskay modified the milestones: Next release, Summer Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants