{teal} module returns a teal_report object that extends from teal_data
#331
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Marcin <[email protected]>
Signed-off-by: Marcin <[email protected]>
Signed-off-by: Marcin <[email protected]>
|
Code Coverage SummaryDiff against mainResults for commit: 6d38378 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 20 suites 38s ⏱️ Results for commit b791939. |
Unit Tests Summary 1 files 20 suites 37s ⏱️ Results for commit b7c4623. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Additional test case details
Results for commit f2b6b10 ♻️ This comment has been updated with latest results. |
Changed names - `teal_reporter@report` -> `teal_reporter@teal_card` - `teal.reporter::doc()` -> `teal.reporter::teal_card()` - `teal.reporter::report()` -> `teal.reporter::teal_card()` So now you can use ```r data <- teal.reporter::teal_report() # teal_report() class data@teal_card # teal_card class teal.reporter::teal_card(data) # teal_card class ``` Also `teal_card` is the creator, the getter and the setter ```r data <- teal.reporter::teal_report() teal_card(data) <- teal_card() # setter + creator teal_card(data) # getter data@teal_card # direct slot access ``` Companion to: - insightsengineering/teal#1542 - insightsengineering/teal.modules.general#885 - insightsengineering/teal.code#256 --------- Signed-off-by: Marcin <[email protected]> Signed-off-by: André Veríssimo <[email protected]> Co-authored-by: André Veríssimo <[email protected]>
pawelru
reviewed
Jun 10, 2025
| #' @examples | ||
| #' # Create a new empty card | ||
| #' report <- teal_card() | ||
| #' class(report) # Check the class of the object |
There was a problem hiding this comment.
Class names are rather internal names. Let's remove it from user-facing docs
Suggested change
| #' class(report) # Check the class of the object |
pawelru
reviewed
Jun 10, 2025
| #' It enables users to create, manipulate, and serialize report-related data efficiently. | ||
| #' | ||
| #' The `teal_card()` function serves two purposes: | ||
| #' 1. When called with a `teal_report` object, it acts as a getter and returns the card slot |
There was a problem hiding this comment.
not only with a teal_report object but that object must be a first argument
pawelru
reviewed
Jun 10, 2025
Comment on lines
+8
to
+10
| #' The `teal_card()` function serves two purposes: | ||
| #' 1. When called with a `teal_report` object, it acts as a getter and returns the card slot | ||
| #' 2. When called with other arguments, it creates a new `teal_card` object from those arguments |
There was a problem hiding this comment.
this adds ambiguity. can we keep it as a constructor only?
pawelru
reviewed
Jun 10, 2025
Comment on lines
+13
to
+18
| # Manual import instead of using backports and adding 1 more dependency | ||
| if (getRversion() < "4.4") { | ||
| `%||%` <- function(x, y) if (is.null(x)) y else x | ||
| assign("`%||%`", `%||%`, envir = getNamespace(pkgname)) | ||
| } | ||
|
|
There was a problem hiding this comment.
we already import rlang - let's take it from there instead of rewriting
pawelru
reviewed
Jun 10, 2025
There was a problem hiding this comment.
- what's our strategy for
eval_code? I thought we are discouraging the usage of this and here I can see the opposite - (related to above) do we plan to have the same for
within?
pawelru
reviewed
Jun 10, 2025
| #' These objects are typically processed later to generate the final R Markdown text. | ||
| #' | ||
| #' @param code A character string containing the R code. | ||
| #' @param ... Additional named parameters to be included as chunk options (e.g., `echo = TRUE`). |
There was a problem hiding this comment.
- please expand on the documentation that these parameters are further used in the markdown code chunks - is it this? https://yihui.org/knitr/options/
- add assert that the list must be named
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
Fixes:
Built on top of:
teal.reporterCards #307teal.reporterCards #307 will be closed once this PR is stable)Companion PRs:
{teal}module returns ateal_reportobject that extends fromteal_datateal#1541{teal}module returns ateal_reportobject that extends fromteal_datateal.code#255{teal}module returns ateal_reportobject that extends fromteal_datateal.data#370{teal}module returns ateal_reportobject that extends fromteal_data#331{teal}module returns ateal_reportobject that extends fromteal_datateal.modules.general#884Changes description
teal_reportthat extendsteal_dataeval_code()andeval_code(cache = TRUE)report(<teal_report>)report_documenttodoc/document/page(final name TBD)