📑 Reporter Refactor: Adds support for new teal_report and teal_card class
#884
Conversation
|
Code Coverage SummaryDiff against mainResults for commit: e48b58a Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 23 suites 2s ⏱️ Results for commit e48b58a. ♻️ This comment has been updated with latest results. |
Companion to insightsengineering/teal.reporter#334 Consequence of changing naming convention for `teal_report` object. --------- Co-authored-by: André Veríssimo <[email protected]>
…l_data` (#255) # Pull Request Fixes: - insightsengineering/teal#1526 Built on top of: - insightsengineering/teal.reporter#307 - _(#307 will be closed once this PR is stable)_ ### Companion PRs: - insightsengineering/teal#1541 - #255 - insightsengineering/teal.data#370 - insightsengineering/teal.reporter#331 - insightsengineering/teal.modules.general#884 ### Changes description - [x] Add new parameter `cache` - Caches the result of the last evaluation in the respective `@code` slot - [ ] Decide on name - [x] Remove signature with multiple arguments to allow overriding `eval_code` in other packages without showing a note ``` r pkgload::load_all("teal.code") #> ℹ Loading teal.code q <- qenv() |> eval_code(1 + 1, cache = TRUE) |> eval_code(mtcars <- head(mtcars)) attr(q@code[[1]], "cache") #> [1] 2 ``` <sup>Created on 2025-06-03 with [reprex v2.1.1](https://reprex.tidyverse.org)</sup> --------- Co-authored-by: Dawid Kaledkowski <[email protected]> Co-authored-by: Marcin <[email protected]> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
…l_data` (#370) # Pull Request Fixes: - insightsengineering/teal#1526 Built on top of: - insightsengineering/teal.reporter#307 - _(#307 will be closed once this PR is stable)_ ### Companion PRs: - insightsengineering/teal#1541 - insightsengineering/teal.code#255 - #370 - insightsengineering/teal.reporter#331 - insightsengineering/teal.modules.general#884 ### Changes description - [x] Cleanup of `teal_data` class to allow for `teal_report` extension - [x] Change the `show()` method to remove reference to `teal_data` specifically --------- Co-authored-by: Dawid Kaledkowski <[email protected]> Co-authored-by: Marcin <[email protected]>
# Pull Request
- Changes from `teal_reportable` branch at `{ŧeal.report}`
- Remove old logic that was defusing the argument
Signed-off-by: Marcin <[email protected]>
Merging this to a feature branch. The rest can be dona as normal issues. Thanks!
Co-authored-by: André Veríssimo <[email protected]> Signed-off-by: Marcin <[email protected]>
Signed-off-by: André Veríssimo <[email protected]>
There was a problem hiding this comment.
-
associationplot is not storing the size of the plot -
bivariateplot is not storing the size of the plot - There's an error in
tm_outliersthat only pops up sometimes with:
I cannot reproduce it consistently, my steps are:
- Open App
- Go to outliers module
- Change width size to 200
- Click on "Add to report"
Warning in .geometry(width, height, units, res) :
NAs introduced by coercion
Warning: Error in .geometry: invalid width
98: stop
97: .geometry
96: grDevices::png
95: .toHTML.recordedplot [/home/averissimo/work/roche🟦/packages📦/teal.reporter/R/toHTML.R#62]
93: toHTML.default [/home/averissimo/work/roche🟦/packages📦/teal.reporter/R/toHTML.R#8]
91: .toHTML.chunk_output [/home/averissimo/work/roche🟦/packages📦/teal.reporter/R/toHTML.R#111]
89: toHTML.default [/home/averissimo/work/roche🟦/packages📦/teal.reporter/R/toHTML.R#8]
87: lapply
86: reporter$append_cards [/home/averissimo/work/roche🟦/packages📦/teal.reporter/R/Reporter.R#63]
85: observe [/home/averissimo/work/roche🟦/packages📦/teal.reporter/R/add_card.R#188]
84: <observer:observeEvent(input$add_card_ok)> [/tmp/Rtmp0ZAG1Z/renv-package-new-41b3c7245b6ce/shiny/R/utils.R#1482]
5: shiny::runApp [/tmp/Rtmp0ZAG1Z/renv-package-new-41b3c7245b6ce/shiny/R/runapp.R#388]
4: eval [/home/averissimo/work/roche🟦/packages📦/teal.modules.general/dev-av/mega_app.R#44]
3: eval
1: sourceI'm trying to tackle these
# Pull Request <!--- Replace `#nnn` with your issue link for reference. --> - Companion of insightsengineering/teal.modules.general#884 --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
|
Solved all bugs with the modules described above. ( Last one before we merge (@m7pr I've re-tested all modules)
|
|
I cannot approve my own PR, but the bug above is on This PR is fine by me, @m7pr please review and give your verdict :) |
|
@averissimo already approved! I see the fix with |
| #' To learn more please refer to the vignette | ||
| #' `vignette("transform-module-output", package = "teal")` or the [`teal::teal_transform_module()`] documentation. | ||
| #' | ||
| #' @inheritSection teal::example_module Reporting |
There was a problem hiding this comment.
@averissimo I get a warning that this section does not exist. I was checking it's companion teal PR where I would have expected this section and it was not there. Am I missing something or do we need to updated the teal::example_module doc?
There was a problem hiding this comment.
I confirm. Section probably got removed here and teal.modules.* can't inherit reporter documentation. I think it is worth to have a # Reporting (and maybe @return) inherited from teal::example_module (or from other place in teal).
P.S. Name of the section could be more universal as SRC is also a concern
There was a problem hiding this comment.
I was trying to track down where this cames from and finally found it in an open PR: https://github.com/insightsengineering/teal/pull/1545/files#diff-30b1b14a6fc7f2b6907f241d0f5d2e780dea7396ded16caffc8147b3d0fdf5a7
The @return tag will be tricky as these functions return the module itself. We could extend it to include what the srv returns
There was a problem hiding this comment.
Maybe something like this?
#' @return
teal_modulewhoseserverreturnsreactive(<teal_data>)...
Pull Request
Fixes:
Built on top of:
teal.reporterCards teal#1499 (will be closed once this PR is stable)Companion PRs:
teal_reportandteal_cardclass teal#1541{teal}module returns ateal_reportobject that extends fromteal_datateal.code#255{teal}module returns ateal_reportobject that extends fromteal_datateal.data#370teal_report,teal_cardclass and revamps reporter UI teal.reporter#331teal_reportandteal_cardclass #884Changes description
teal_reportobject on every module