easystats / insight

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

Formatting issue with print_md() applied to compare_parameters() #887

Closed jmgirard closed 1 day ago

jmgirard commented 5 months ago

The formatting for print_md() applied to compare_parameters() differs from that when applied to compare_performance(). As a result, the former is not displaying properly in interactive mode in RStudio.

library(easystats)
#> # Attaching packages: easystats 0.7.2
#> ✔ bayestestR  0.13.2   ✔ correlation 0.8.4 
#> ✔ datawizard  0.11.0   ✔ effectsize  0.8.8 
#> ✔ insight     0.20.0   ✔ modelbased  0.8.7 
#> ✔ performance 0.12.0   ✔ parameters  0.21.7
#> ✔ report      0.5.8    ✔ see         0.8.4
fit1 <- lm(Sepal.Length ~ Species, data = iris)
fit2 <- lm(Sepal.Width ~ Species, data = iris)
compare_parameters(fit1, fit2) |> print_md()
| Parameter            | fit1              | fit2                 |
|----------------------|-------------------|----------------------|
| (Intercept)          | 5.01 (4.86, 5.15) | 3.43 (3.33, 3.52)    |
| Species (versicolor) | 0.93 (0.73, 1.13) | -0.66 (-0.79, -0.52) |
| Species (virginica)  | 1.58 (1.38, 1.79) | -0.45 (-0.59, -0.32) |
|                      |                   |                      |
| Observations         | 150               | 150                  |
compare_performance(fit1, fit2) |> print_md()
#> When comparing models, please note that probably not all models were fit
#>   from same data.
| Name | Model | AIC (weights) | AICc (weights) | BIC (weights) | R2 | R2 (adj.) | RMSE | Sigma |
|:---|:--:|:--:|:--:|:--:|:--:|:--:|:--:|:--:|
| fit1 | lm | 231.5 (\<.001) | 231.7 (\<.001) | 243.5 (\<.001) | 0.62 | 0.61 | 0.51 | 0.51 |
| fit2 | lm | 106.7 (\>.999) | 107.0 (\>.999) | 118.8 (\>.999) | 0.40 | 0.39 | 0.34 | 0.34 |

Comparison of Model Performance Indices

Created on 2024-06-10 with reprex v2.1.0

image

strengejacke commented 5 months ago

Yes, this started as an experimental feature, because I tested the tinytable package as alternative. Thus, some functions now support different engines for printing to HTML or MD. Use the engine argument to specify whether you want to use tinytable for HTML and MD, or gt for HTML and insight for MD. In this case, simply use engine == "tt" or engine = "default". I should document this argument.

strengejacke commented 5 months ago

Just a question: which output is not correct in which context?

jmgirard commented 5 months ago

For my purposes at least, the second version (compare_performance's) is much better.

strengejacke commented 5 months ago

But the rendered file looks ok? Or do you have in general problems with markdown rendering for compare_parameters() |> print_md()? Furthermore, could you try compare_parameters() |> print_md(engine = "default")?

strengejacke commented 5 months ago

@vincentarelbundock Do you know why the table is not correctly rendered in R notebooks on preview when using tinytable for markdown tables?

See screenshot:

image

Rendering works well for the final compiled notebook/HTML, but not for the preview inside RStudio.

vincentarelbundock commented 5 months ago

Not sure. What tinytable commanda do you use internally?

strengejacke commented 5 months ago
  # create base table
  out <- tinytable::tt(formatted_table, notes = footer, caption = caption)
  # insert sub header rows and column spans, if we have them
  if (!(is.null(row_groups) && is.null(col_groups))) {
    out <- tinytable::group_tt(out, i = row_groups, j = col_groups)
  }
  out@output <- outformat
  out

Export function code starts here.

strengejacke commented 5 months ago

print_md() calls that function to create the tiny table at the end: https://github.com/easystats/parameters/blob/5adbe971708954e80e6f0e2d505e549862875ca0/R/print_md.R#L128

vincentarelbundock commented 5 months ago

At the end of this function, it looks like you are overriding the output format determined by tinytable. If that's the case, then you have essentially taken over responsibility for detecting the appropriate output format...

https://github.com/easystats/parameters/blob/5adbe971708954e80e6f0e2d505e549862875ca0/R/print_md.R#L451C1-L451C26

strengejacke commented 5 months ago

I set the output format explicitly to markdown, that's why I expected it to work correctly (it does when rendering the notebook, but not if you click that "preview arrow" in the chunk).

This is what I get when I do not override the output format, the table is printed as HTML in the viewer:

image

vincentarelbundock commented 5 months ago

You should not manually override the table format. Forcing rendering to go through markdown may work for simple tables, but it disables a lot of features like colors and styles. So your tables are not as flexible as they could be.

Rendering HTML tables inside a notebook preview should be handled upstream. I opened an issue here:

https://github.com/vincentarelbundock/tinytable/issues/282

I'll have to look into it, because I'm not quite sure how these previews are handled by RStudio. Should I just return HTML? How can I distinguish cases where a user wants to embed in the preview or wants to run on the console and print to the viewer?

I'm completely overwhelmed with work now, so might not get to this super quickly, unfortunately.

strengejacke commented 5 months ago

You should not manually override the table format. Forcing rendering to go through markdown may work for simple tables, but it disables a lot of features like colors and styles. So your tables are not as flexible as they could be.

I know, but that's intentional. We have print_md() that should create pure markdown tables, and print_html(), which provides more flexibility. The print() methods in easystats are supposed to satisfy most basic needs. For more complex table, we would refer to tinytable, modelsummary, gt or gtsummary.

The reason why we override the output format is because else we could not create HTML tables with tinytable from interactive/console, when it's requested (via print_html()).

strengejacke commented 5 months ago

I'll have to look into it, because I'm not quite sure how these previews are handled by RStudio. Should I just return HTML? How can I distinguish cases where a user wants to embed in the preview or wants to run on the console and print to the viewer?

I think it's not high priority, because we can use print_md(engine = "default"), which "resolves" the issue. But I'm also not sure how RStudio handles Notebook previews, and what would be the best solution, or if you have control about this inside your own package at all?

vincentarelbundock commented 5 months ago

cool cool. Let's leave this open, because it would be very nice to have proper display in notebooks. I'll circle back to this when I have time to investigate.