Skip to content

Vignettes #1545

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

Open
wants to merge 26 commits into
base: teal_reportable
Choose a base branch
from
Open

Vignettes #1545

wants to merge 26 commits into from

Conversation

m7pr
Copy link
Contributor

@m7pr m7pr commented Jun 6, 2025

Extends teal vignettes with new reporting features in teal.

@m7pr m7pr requested a review from averissimo June 6, 2025 13:37
@m7pr m7pr added the core label Jun 6, 2025
@gogonzo gogonzo self-assigned this Jun 9, 2025
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. "Adding Support for Reporting to Custom Modules" uses old solution with reporter arg in the $server
  2. "getting started with teal" has very short subsections which links to longer documentations. Reporter here adds extra 100 lines which changes look of the vignette so much. IMO describing possible values of init(reporter) can be shorter just bullet-points instead of sections. Maybe we should change "Adding Support for Reporting to Custom Modules" to "Creating reproducible reports" (name tbd) and describe everything related to report there?

@m7pr
Copy link
Contributor Author

m7pr commented Jun 9, 2025

@gogonzo about

Maybe we should change "Adding Support for Reporting to Custom Modules" to "Creating reproducible reports" (name tbd) and describe everything related to report there?

Do you think this should be a document in teal or in teal.reporter? I also have a feeling that this needs a special document that covers only reporting, which is now very extensive : )

@gogonzo
Copy link
Contributor

gogonzo commented Jun 9, 2025

@gogonzo about

Maybe we should change "Adding Support for Reporting to Custom Modules" to "Creating reproducible reports" (name tbd) and describe everything related to report there?

Do you think this should be a document in teal or in teal.reporter? I also have a feeling that this needs a special document that covers only reporting, which is now very extensive : )

We can do this:

  • change a content "Adding Support for Reporting to Custom Modules" to fit a new design. This vignette is also linked in "Creating custom modules", so I guess it is ok to keep it, but update the content
  • Create a new vignette called "Managing Reproducible Report Documents in Teal" where we describe:
    a. init(reporter) to control initial state of the reporter
    b. Managing teal_module(s) report cards (with disable_report and after)

@m7pr
Copy link
Contributor Author

m7pr commented Jun 9, 2025

@gogonzo make sense!
Do we need special vignette to cover teal_report? Or can it be included in "Adding Support for Reporting to Custom Modules"?

@gogonzo
Copy link
Contributor

gogonzo commented Jun 9, 2025

@gogonzo make sense! Do we need special vignette to cover teal_report? Or can it be included in "Adding Support for Reporting to Custom Modules"?

We for sure need teal_report vignette in teal.report. I don't know where in teal should we mention teal_report. Maybe in "Include Data in Teal Application"? I wouldn't spend too much time about teal_report as it serves the same role as teal_data and also it should be possible to specify init(data = <teal_report>).

@m7pr
Copy link
Contributor Author

m7pr commented Jun 9, 2025

Ok, I:

  • simplified and shortened teal_report Class in teal.reporter
  • simplified and shortened Getting started with teal.modules.general in teal
  • updated Adding Support for Reporting to Custom Modules in teal
  • created Managing Reproducible Report Documents in teal in teal

Left 3 TODO in Managing Reproducible Report Documents in teal in teal

Comment on lines +104 to +106
tcard <- c("Templated information", card)
attributes(tcard) <- attributes(card)
tcard
Copy link
Contributor Author

@m7pr m7pr Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nasty. c removes all classes and attributes. We can substitute this example to use edit_teal_card.
So it's

custom_reporter_template$set_template(
  function(card) {
    edit_teal_card(card, append = "Templated information", after = 0)
  }
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use append(card, <something to add>, after = 0) - then it doesn't loose a class because a first argument is a card

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think I'll go with this one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gogonzo append doesn't loose the class, but it looses metadata attribute, so there is no name to be displayed in Reporter previewer.

> custom_card <- teal_card()
> metadata(custom_card, "title") <- "The title"
> custom_card
An object of class "teal_card"
Slot "metadata":
Slot "metadata":$title
[1] "The title"

> append(custom_card, "x")
[[1]]
[1] "x"

> append(custom_card, teal_card("x"))
[[1]]
[1] "x"

attr(,"class")
[1] "teal_card"
> metadata(append(custom_card, teal_card("x")))
list()

What do you think about exposing append.teal_card method that also keeps attributes?
We can actually just change edit_teal_card to append.teal_card and remove the part used to modify

image

See below:

```{r module_2}
my_module_with_reporting <- function(label = "example teal module") {
module(
label = label,
server = function(id, data, reporter) {
server = function(id, data) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module returns unmodified data while output shows the selected dataset.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gogonzo for pointing this out. I wanted to keep the example easy, so we just show that it is enough to return(data) to turn reporting on.

We can extend this example module by adding

rdata <- reactive({
  eval_code(
    data(), 
    substitute(head(dataname), env = list(dataname = as.name(input$dataname))), 
    keep_output = TRUE
  )
})
return(rdata)

or

rdata <- reactive({
  req(input$dataname)
  obj <- data()
  teal_card(obj) <- c(teal_card(obj), "## Glimpse of the dataset", head(obj[[input$dataname]]))
  obj
})

return(rdata)

depending on whether we would like to build the report with eval_code or with teal_card.
I think eval_code is more appropriate here, even though we have substitute usage in it (that I think is advanced for regular R users). We can also extend this example by explaining the keep_output = TRUE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However this very same code is added in the next section Add content to the report card

So maybe keep Modify the body of the server function as it is, and extend Add content to the report card to use eval_code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this vignette needs to have two examples. Previously adding reporter was more complicated now Reporter is supported by default as teal_report is passed to the teal_module's server. Only thing which user has to do is return(<teal_report>). I'd just mention keep_output and add one markdown-heading element.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I can remove the whole first example and just use the last one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplified adding-support-for-reporting.Rmd vignette 2cca04b

custom_reporter_template <- Reporter$new()
custom_reporter_template$set_template(
# Need to return named `teal_card`
function(card) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show some practical information, like setting a Rmd template or adding some header/footer etc.?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was rather thinking about setting some templated text for disclaimers, data sources, data privacy information, etc as an information added on top or at the bottom of the card. This sentence can also be added to this example.

The Rmd template could be useful, if we create some text that can be interpreted as rmarkdown. Maybe including a logo that is taken from the web?

Comment on lines +104 to +106
tcard <- c("Templated information", card)
attributes(tcard) <- attributes(card)
tcard
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use append(card, <something to add>, after = 0) - then it doesn't loose a class because a first argument is a card


#### Module-Level Reporting Control

While the `reporter` parameter controls reporting globally, you can also disable reporting for specific modules using `disable_report()`. This allows you to have fine-grained control over which modules support reporting:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both, after and disable_report can control module's report. After is more powerful and it doesn't restrict to the report only but to the whole teal_report/teal_data object. We can showcase how to modify the report using after

Suggested change
While the `reporter` parameter controls reporting globally, you can also disable reporting for specific modules using `disable_report()`. This allows you to have fine-grained control over which modules support reporting:
While the `reporter` parameter controls reporting globally, you can also manage a report document for a specific module using `after()` `disable_report()`:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gogonzo there is a section following this sentence that presents the usage of after:
Using the after Function to Customize Module Report Cards

Do you think it's enough? It shows that you can adjust the teal_card of teal_report, but we can apply join_keys modifications as well, and also an eval_code to show that the whole teal_data/teal_report can be changed in any direction, not only the @teal_report field.

m7pr and others added 5 commits June 10, 2025 09:49
Co-authored-by: Dawid Kałędkowski <dawid.kaledkowski@gmail.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: Dawid Kałędkowski <dawid.kaledkowski@gmail.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: Dawid Kałędkowski <dawid.kaledkowski@gmail.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: Dawid Kałędkowski <dawid.kaledkowski@gmail.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: Dawid Kałędkowski <dawid.kaledkowski@gmail.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
m7pr and others added 3 commits June 10, 2025 12:20
Co-authored-by: Dawid Kałędkowski <dawid.kaledkowski@gmail.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
m7pr and others added 5 commits June 11, 2025 09:02
Co-authored-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants