stanreg, brmsfit tidiers are inconsistent with tidyMCMC

[jacob-long]

I see that @paul-buerkner wrote the method for brmsfit objects and I can anticipate some of the reasons he may have done this intentionally, but I'm not sure whether the current format makes sense.

While some aspects (par_type, parameters, etc.) are related to broader issues with mixed models (e.g., #96), that's not what I'm worried about since there are ongoing discussions about that. Instead, I'm concerned about the "confidence" intervals.

tidyMCMC is basically consistent with the general templates: we have conf.int and conf.level arguments and it returns columns with names conf.low and conf.high.

tidy.brmsfit and tidy.stanreg, on the other hand, use intervals and prob arguments and return corresponding columns called lower and upper.

I would say that at minimum, the inconsistency between tidyMCMC and tidy.brmsfit/tidy.stanreg doesn't make sense. Now I suspect tidy.brmsfit/tidy.stanreg opted not use any reference to "conf" because the intervals are not really confidence intervals. My opinion is that it would be better to stick with the broom norms (using the "conf" prefix) since users should understand the distinction and it is better than breaking any function that relies on the generality of tidy (like the one in my package that led me to realize this issue). I assume there are other model types supported by broom using "conf" that produce intervals that are also not truly confidence intervals, but I'm not certain.

But if others disagree with me about the nomenclature, I think it would then be best to change tidyMCMC so that someone who is willing to deal with MCMC models in this specific way doesn't have to switch back and forth between naming schemes to support both brmsfit/stanreg and stanfit/rjags/whatever else objects.

[alexpghayes]

I would love to use lower and upper for intervals in general, but we're probably too locked into conf.low and conf.high at this point. I think we should probably use conf.low and conf.high for all intervals throughout broom where it isn't egregiously misleading.

Will come back to this later after there's been time for people to discuss.

[alexpghayes]

This discussion should also happen in broom.mixed since most of the Bayesian tidiers are migrating over there. Linking to bbolker/broom.mixed#2.

[bbolker]

I think this can be closed. tidy.brmsfit and tidy.stanreg are now more broom-compliant. I did discuss this with @paul-buerkner and he seemed OK with it; I haven't spoken to the original stanreg authors, I hope they're not too unhappy. In any case, if this is still/becomes an issue, it should be re-opened in the broom.mixed issues list.

[github-actions]

This issue has been automatically closed due to inactivity.

[github-actions]

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.