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

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

Open
jmgirard opened this issue Jun 10, 2024 · 14 comments · Fixed by easystats/parameters#979
Open
Labels
3 investigators ❔❓ Need to look further into this issue Low priority 😴 This issue can be easily workaround or happens only in edge cases

Comments

@jmgirard
Copy link

jmgirard commented Jun 10, 2024

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

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 added a commit to easystats/parameters that referenced this issue Jun 10, 2024
* Formatting issue with print_md() applied to compare_parameters()
Fixes easystats/insight#887

* docs

* docs

* docs

* dont use remotes

* lintr, docs

* docs

* fix
@strengejacke strengejacke reopened this Jun 10, 2024
@strengejacke
Copy link
Member

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

@jmgirard
Copy link
Author

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

@strengejacke
Copy link
Member

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

strengejacke commented Jun 11, 2024

@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
Copy link
Contributor

Not sure. What tinytable commanda do you use internally?

@strengejacke
Copy link
Member

  # 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
Copy link
Member

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

@vincentarelbundock
Copy link
Contributor

vincentarelbundock commented Jun 11, 2024

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:/easystats/parameters/blob/5adbe971708954e80e6f0e2d505e549862875ca0/R/print_md.R#L451C1-L451C26

@strengejacke
Copy link
Member

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

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:

vincentarelbundock/tinytable#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
Copy link
Member

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

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?

@strengejacke strengejacke added Low priority 😴 This issue can be easily workaround or happens only in edge cases 3 investigators ❔❓ Need to look further into this issue labels Jun 23, 2024
@vincentarelbundock
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 investigators ❔❓ Need to look further into this issue Low priority 😴 This issue can be easily workaround or happens only in edge cases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants