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.

Sign up for GitHub

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

Add ability to set datanames #824

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+33 −5
Open

Add ability to set datanames #824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9d52672
Add datasets_selected argument
llrs-roche Jan 8, 2025
4cab9d5
Use datasets_selected for datanames
llrs-roche Jan 8, 2025
01af1e5
Add datasets_selected
llrs-roche Jan 8, 2025
7d1d1e9
Improve checks on datasets_selected
llrs-roche Jan 8, 2025
f64f3a0
Rstudio new version adds an id
llrs-roche Jan 8, 2025
f23555d
[skip roxygen] [skip vbump] Roxygen Man Pages Auto Update
github-actions[bot] Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions R/tm_front_page.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#' @param footnotes (`character` vector) of text to be shown at the bottom of the module, for each
#' element, if named the name is shown first in bold, followed by the value.
#' @param show_metadata (`logical`) indicating whether the metadata of the datasets be available on the module.
#' @inheritParams tm_variable_browser
#'
#' @inherit shared_params return
#'
Expand Down Expand Up @@ -69,7 +70,8 @@ tm_front_page <- function(label = "Front page",
tables = list(),
additional_tags = tagList(),
footnotes = character(0),
show_metadata = FALSE) {
show_metadata = FALSE,
datasets_selected = character(0L)) {
message("Initializing tm_front_page")

# Start of assertions
Expand All @@ -79,18 +81,27 @@ tm_front_page <- function(label = "Front page",
checkmate::assert_multi_class(additional_tags, classes = c("shiny.tag.list", "html"))
checkmate::assert_character(footnotes, min.len = 0, any.missing = FALSE)
checkmate::assert_flag(show_metadata)
checkmate::assert_character(datasets_selected, min.len = 0, min.chars = 1)

# End of assertions

# Make UI args
args <- as.list(environment())
datanames <- if (show_metadata && length(datasets_selected) > 0L) {
datasets_selected
} else if (show_metadata) {
"all"
} else {
NULL
}
Comment on lines +90 to +96
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a gap here when show_metadata = FALSE and length(datasets_selected). This show_metadata argument is unnecessary and I would suggest to deprecate show_metadata and to show metadata when datasets_selected is not empty. So:

tm_front_page <- function(
  datasets_selected = NULL,
  show_metadata = <deprecated>
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On that condition show_metadata prevails and nothing would be shown... but I can deprecate show_metadata entirely.


ans <- module(
label = label,
server = srv_front_page,
ui = ui_front_page,
ui_args = args,
server_args = list(tables = tables, show_metadata = show_metadata),
datanames = if (show_metadata) "all" else NULL
datanames = datanames
)
attr(ans, "teal_bookmarkable") <- TRUE
ans
Expand Down
6 changes: 5 additions & 1 deletion R/tm_missing_data.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#'
#' @inheritParams teal::module
#' @inheritParams shared_params
#' @inheritParams tm_data_table
#' @param parent_dataname (`character(1)`) Specifies the parent dataset name. Default is `ADSL` for `CDISC` data.
#' If provided and exists, enables additional analysis "by subject". For non-`CDISC` data, this parameter can be
#' ignored.
Expand Down Expand Up @@ -110,6 +111,7 @@
tm_missing_data <- function(label = "Missing data",
plot_height = c(600, 400, 5000),
plot_width = NULL,
datasets_selected = character(0L),
Copy link
Contributor

Choose a reason for hiding this comment

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

if this argument was named datanames it would be inherited from teal::module.

image

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 went with what is used in other modules, currently there are 2 modules using this argument: tm_data_table and tm_variable_browser. Let me know if you want me to rename those other modules too.
I haven't tested what happens with transformators...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be much easier if tm_data_table and tm_variable_browser had datanames instead of datasets_selected . I wouldn't rename them, but:

  datanames = "all",
  datasets_selected = <deprecated>

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 will do that and check what happens with transformators.

parent_dataname = "ADSL",
ggtheme = c("classic", "gray", "bw", "linedraw", "light", "dark", "minimal", "void"),
ggplot2_args = list(
Expand Down Expand Up @@ -143,6 +145,7 @@ tm_missing_data <- function(label = "Missing data",
lower = plot_width[2], upper = plot_width[3], null.ok = TRUE, .var.name = "plot_width"
)

checkmate::assert_character(datasets_selected, min.len = 0, min.chars = 1)
checkmate::assert_character(parent_dataname, min.len = 0, max.len = 1)
ggtheme <- match.arg(ggtheme)

Expand All @@ -158,9 +161,11 @@ tm_missing_data <- function(label = "Missing data",
assert_decorators(decorators, null.ok = TRUE, names = available_decorators)
# End of assertions

datasets_selected <- unique(datasets_selected)
ans <- module(
label,
server = srv_page_missing_data,
datanames = if (length(datasets_selected) == 0) "all" else datasets_selected,
server_args = list(
parent_dataname = parent_dataname,
plot_height = plot_height,
Expand All @@ -170,7 +175,6 @@ tm_missing_data <- function(label = "Missing data",
decorators = decorators
),
ui = ui_page_missing_data,
datanames = "all",
ui_args = list(pre_output = pre_output, post_output = post_output)
)
attr(ans, "teal_bookmarkable") <- TRUE
Expand Down
2 changes: 1 addition & 1 deletion R/tm_variable_browser.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ tm_variable_browser <- function(label = "Variable Browser",
label,
server = srv_variable_browser,
ui = ui_variable_browser,
datanames = "all",
datanames = if (length(datasets_selected) == 0) "all" else datasets_selected,
server_args = list(
datasets_selected = datasets_selected,
parent_dataname = parent_dataname,
Expand Down
8 changes: 7 additions & 1 deletion man/tm_front_page.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions man/tm_missing_data.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions teal.modules.general.Rproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Version: 1.0
ProjectId: da9d7aac-6635-4d40-a6f3-881a397ffae1

RestoreWorkspace: No
SaveWorkspace: No
Expand Down
Loading