rfortherestofus / omni

RMarkdown template, ggplot2 theme, and table function for OMNI Institute
https://rfortherestofus.github.io/omni/
Other
21 stars 4 forks source link

Revisiting adding a caption to omni_table() #63

Closed OskaratOmni closed 8 months ago

OskaratOmni commented 8 months ago

Hey @tvroylandt and @dgkeyes
The topic of having an easy method to add a caption to an omni table has come up again in the last couple of days.

One thing that is nice about a table with a caption is that it facilitates automatic table numbering and referencing. There are ways to append a caption to a call to omni_table(). (David shared some code to do this a while back). I have done this for Word outputs and I'm sure it is easy to apply to html outputs.

However, for ease of use, it might be nice if there was an argument to omni_table() function such that the default was NULL but if you add some text then it applies that text to table.

So perhaps tabledata |> omni_table() makes the normal table but tabledata |> omni_table(caption = "This is a caption") makes a table with caption.

What do you think? Is this an annoying feature to add or fairly reasonable? If you know another way to achieve automatic numbering and referencing then we could discuss that as well - this just seems the most straight forward. In the past, I have also used html to add captions but that's a bit tedious.

Please let me know what you think. thanks!

OskaratOmni commented 8 months ago

@tvroylandt I was just following up on this. I know you might be on holiday and are very busy with other things. Do you think you could have time to look at this next week?

tvroylandt commented 8 months ago

I'll put that on my to do list for this week

tvroylandt commented 8 months ago

I've added caption for both gt and flextable option for HTML and WOrd output (only flextable for Word). Please see the above commits for change. Would that fit @OskaratOmni ?

OskaratOmni commented 8 months ago

Hey @tvroylandt - this seems good. I have only seen the code but have not tried it. Have these commits been pushed? I have just loaded the OMNI Package, and the caption argument is visible, but I get an error when I use it. Do I need to specify the output option?

compare: diamonds |> group_by(cut) |> summarize(mean_price = mean(price)) |> omni_table() to: diamonds |> group_by(cut) |> summarize(mean_price = mean(price)) |> omni_table(caption = "Mean Diamond Prices")

dgkeyes commented 8 months ago

I can see that @tvroylandt pushed the commits on this. Just want to make sure that you reinstalled the package @OskaratOmni.

dgkeyes commented 8 months ago

@tvroylandt I'm seeing an error on this. Here's my Rmd file:

---
title: "Untitled"
author: "David Keyes"
date: "2024-01-17"
output: word_document
---

```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = TRUE)
library(omni)
library(tidyverse)
diamonds |>
  group_by(cut) |>
  summarize(mean_price = mean(price)) |>
  omni_table()
diamonds |>
  group_by(cut) |>
  summarize(mean_price = mean(price)) |>
  omni_table(caption = "Mean Diamond Prices")


The caption doesn't show up correctly in Word. Instead, I see this ("caption" instead of "Mean Diamond Prices"):

![CleanShot 2024-01-17 at 08 52 25](https://github.com/rfortherestofus/omni/assets/13971585/e5bd64d9-508e-42f6-8774-d8f455e2096b)
OskaratOmni commented 8 months ago

I can see that @tvroylandt pushed the commits on this. Just want to make sure that you reinstalled the package @OskaratOmni.

Yes, of course.

dgkeyes commented 8 months ago

Hmm, strange. Can you do a video recording of what you're seeing?

On this question:

I have just loaded the OMNI Package, and the caption argument is visible, but I get an error when I use it. Do I need to specify the output option?

Thomas wrote the code so that it checks the output format automatically and adjusts based on that. You shouldn't need to specify it yourself.

OskaratOmni commented 8 months ago

@dgkeyes
just from the above comment - i cannot run this code: diamonds |> group_by(cut) |> summarize(mean_price = mean(price)) |> omni_table(caption = "Mean Diamond Prices") from the console. First the error said 'caption' was an unused argument, which was weird because it was in the help file for omni_table and visible as an argument in the function.

Now it still returns an error (I did update some packages in between), but a different one: Error in if (knitr::opts_knit$get("rmarkdown.pandoc.to") == "docx") { : argument is of length zero

dgkeyes commented 8 months ago

Weird. I can't replicate this issue. Here's what I see: https://show.rfor.us/2nj5PYtW

Do you want to record a video to show me what you see? https://rfortherestofus.com/send-video/

OskaratOmni commented 8 months ago

Hey David, I can record a video but what if you just place your cursor to the left of: diamonds |> group_by(cut) |> summarize(mean_price = mean(price)) |> omni_table(caption = "Mean Diamond Prices") and hit control + return

do you see the table in the viewer?

dgkeyes commented 8 months ago

Yes, you're right that it doesn't work in this way. That's because the omni_table() function is now checking to see what the output format of your RMarkdown document is and adjusting what it does based on that. If you try to just run the code (i.e. working interactively), then there is no output format so it throws an error. We could perhaps adjust the function so it works differently if you're working interactively and shows the table, but it's never going to show the caption when you work interactively because that is only added during the knitting process. Does that make sense?

OskaratOmni commented 8 months ago

Yes, that's basically what I was assuming from the error message and the behavior.

Without being able to run it from the console, or code chunk, I'm not sure we will use it very much. That said, it will still be a useful feature to add at the last stage of creating a knitted file, so let's keep it.

tvroylandt commented 8 months ago

@dgkeyes @OskaratOmni I've added something so that it should work outside of an Rmd file. Could you confirm this is ok on your side ?

OskaratOmni commented 8 months ago

I just tested this and yes works great, thanks!!