easystats / insight

:crystal_ball: Easy access to model information for various model objects
https://easystats.github.io/insight/
GNU General Public License v3.0
405 stars 39 forks source link

`digits` arguments in format_*() functions rounding rather than sigfigs #383

Closed NFA closed 3 years ago

NFA commented 3 years ago

Hello,

You are currently not exposing a digits argument to the hdi function to specify how many significant digits to display (would be nice). However, the print method for the bayestestR_hdi class has a digitsargument with a default of 2 which is forwarded to the .print_default but without the expected result. I noticed this because I have a posterior distribution with relative small numbers and the median was close to the lower limit which didn't seem accurate. That's when I noticed there was rounding in the output.

I attach short output illustrating the issue, and expected output with an option of digits = 2.

Thanks for a nice package!

> median(diffs$diffED)
[1] 0.01035668
> interval <- hdi(diffs$diffED)
> interval
95% HDI: [0.01, 0.02]
> print.data.frame(interval)
    CI      CI_low    CI_high
1 0.95 0.005325208 0.01527145
> print.data.frame(interval, digits = 2)
    CI CI_low CI_high
1 0.95 0.0053   0.015
>
bwiernik commented 3 years ago
set.seed(123)
posterior <- rnorm(1000) / 1000
bayestestR::hdi(posterior, ci = c(.80, .90, .95)) |> print(ci_digits = 2)
#> Highest Density Interval
#> 
#> 80% HDI       |       90% HDI |       95% HDI
#> ---------------------------------------------
#> [ 0.00, 0.00] | [ 0.00, 0.00] | [ 0.00, 0.00]

Created on 2021-06-04 by the reprex package (v2.0.0)

@mattansb @strengejacke @DominiqueMakowski @IndrajeetPatil

I agree here. I don't think that the above is great behavior. I would prefer that we follow R's lead and treat digits as indicating the number of significant digits (eg, using format() or formatC()) rather than rounding. At a minimum, I think we should ensure that at least 1 non-zero digit is shown.

Alternatively, if we want to keep an option to treat digits as rounding rather than significant figures, I think we should provide an argument to get the sigfig behavior.

(One other thing, I think it would be good if we set the default value for ci_digits and rope_digits to be digits so that users can set digits and get the same value for all 3.)

bwiernik commented 3 years ago

(Moving this issue to the relevant repository.

strengejacke commented 3 years ago

Maybe we just need to update the print-method, to use format_value or insight::format_table?

insight::format_value(0.000043, digits = 2)
#> [1] "4.30e-05"
insight::format_value(0.000043, digits = 10)
#> [1] "0.0000430000"

Created on 2021-06-05 by the reprex package (v2.0.0)

strengejacke commented 3 years ago

(I'd say this is a bayestestR issue)

strengejacke commented 3 years ago

(I'd say this is a bayestestR issue)

No, this is actually an issue in format_ci(), which rounds the values when the width argument is "auto":

https://github.com/easystats/insight/blob/8c950dc91e152b63ad918443db0f7375e6241fb8/R/format_ci.R#L56-L60

strengejacke commented 3 years ago

Maybe we can replace round() with format_value()?

mattansb commented 3 years ago

(One other thing, I think it would be good if we set the default value for ci_digits and rope_digits to be digits so that users can set digits and get the same value for all 3.)

Def agree with this.

strengejacke commented 3 years ago

Maybe we can replace round() with format_value()?

This actually didn't work as expected. I have added another option for digits now. What about this?

set.seed(123)
posterior <- rnorm(1000) / 1000
bayestestR::hdi(posterior, ci = c(.80, .90)) |> print(ci_digits = 2)
#> Highest Density Interval
#> 
#> 80% HDI       |       90% HDI
#> -----------------------------
#> [ 0.00, 0.00] | [ 0.00, 0.00]

bayestestR::hdi(posterior, ci = c(.80, .90)) |> print(ci_digits = 6)
#> Highest Density Interval
#> 
#> 80% HDI               |               90% HDI
#> ---------------------------------------------
#> [-0.001265, 0.001263] | [-0.001618, 0.001688]

bayestestR::hdi(posterior, ci = c(.80, .90)) |> print(ci_digits = "scientific")
#> Highest Density Interval
#> 
#> 80% HDI                 |                 90% HDI
#> -------------------------------------------------
#> [-1.265e-03, 1.263e-03] | [-1.618e-03, 1.688e-03]

bayestestR::hdi(posterior, ci = c(.80, .90)) |> print(ci_digits = "scientific6")
#> Highest Density Interval
#> 
#> 80% HDI                       |                       90% HDI
#> -------------------------------------------------------------
#> [-1.265396e-03, 1.263185e-03] | [-1.617883e-03, 1.687589e-03]

bayestestR::hdi(posterior, ci = c(.80, .90)) |> print(ci_digits = "signif")
#> Highest Density Interval
#> 
#> 80% HDI           |           90% HDI
#> -------------------------------------
#> [-0.0013, 0.0013] | [-0.0016, 0.0017]

bayestestR::hdi(posterior, ci = c(.80, .90)) |> print(ci_digits = "signif6")
#> Highest Density Interval
#> 
#> 80% HDI                  |                   90% HDI
#> ----------------------------------------------------
#> [-0.0012654, 0.00126319] | [-0.00161788, 0.00168759]

Created on 2021-06-22 by the reprex package (v2.0.0)

bwiernik commented 3 years ago

Sure.