Code review

DESCRIPTION

@@ -10,10 +10,10 @@ Authors@R:
       person('Luis', 'Morís Fernández', email = '[email protected]', role = c('aut')),
       person('Jessica', 'Knizia', email = '[email protected]', role = c('ctb'))
     )
-Description: DaVinci’s dv.listings module displays a data listing of different dataset domains in tabular form. 
+Description: DaVinci’s dv.listings package provides a modules to display different dataset domains as listings. 
     Users can select the dataset to be shown, and specify which columns of the dataset should be displayed.
     The displayed columns can be sorted and filtered.
-    Moreover, the listing can be filtered to show only entries containing a keyword entered by the user.
+    Moreover, the listing can be filtered to show only entries containing a keyword specified by the user.
     The module is not limited to usage of one data source, most notably, it can handle data from ADaM or SDTM. 
 License: Apache License (>= 2)
 Encoding: UTF-8
@@ -41,7 +41,7 @@ Imports:
     shinyWidgets (>= 0.8.0),
     tibble (>= 3.2.1),
     dv.manager (>= 2.0.0-17)
-RoxygenNote: 7.3.0
+RoxygenNote: 7.3.1
 Roxygen: list(markdown = TRUE)
 Remotes: boehringer-ingelheim/dv.manager@main
 VignetteBuilder: knitr

NAMESPACE

@@ -4,8 +4,6 @@ export("%>%")
 export(convert_data)
 export(listings_UI)
 export(listings_server)
-export(mock_listings_mm)
-export(mock_simple_listing_mm)
 export(mod_listings)
 import(shiny)
 importFrom(magrittr,"%>%")

NEWS.md

@@ -1,6 +1,6 @@
 # dv.listings 3.1.0
 
-package was renamed to dv.listings
+Package was renamed to dv.listings.
 
 # dv.table 3.0.1
 

R/helpers.R

@@ -8,8 +8,6 @@
 #' "No label" for missing labels.
 #'
 #' @keywords internal
-#'
-
 get_labels <- function(dataset) {
   # Catch special cases (e.g. when global filter returns empty dataset)
   return_null <- any(length(dataset) == 0, is.null(dataset))
@@ -55,7 +53,6 @@ get_labels <- function(dataset) {
 #' @return Character vector of the type `<name> [<label>]`. Adds "No label" as placeholder for missing labels.
 #'
 #' @keywords internal
-#'
 generate_choices <- function(dataset) {
   # No checkmate checks since it is catched in get_labels()
 
@@ -79,7 +76,7 @@ generate_choices <- function(dataset) {
 #'   datasets.
 #' @param dataset `[list(data.frame)]`
 #'
-#' A list of data.framish dataset(s) that will be shown in tabular format.
+#' A list of data.framish dataset(s) that will be shown as listings.
 #'
 #' @return Named list of character vectors which Contain the names of columns to be displayed at module launch for
 #'   every dataset. List entries are named according to the \code{dataset} names.
@@ -135,7 +132,6 @@ fill_default_vars <- function(default_vars, dataset) {
 #'   vector of column names created by combining the variable names with their labels.
 #'   \code{row_names} is a character vector containing the number of the corresponding row.
 #' @keywords internal
-#'
 set_data <- function(base_data, selector) {
   # Check validity of parameters
   checkmate::assert(
@@ -201,7 +197,7 @@ set_labels <- function(dataset, labels) {
 #' Converts data types of data.frame columns to logical, integer, numeric, complex, character or factor as appropriate.
 #' Re-assigns column labels after converting data type.
 #'
-#' @param dataset  `[data.frame]`
+#' @param dataset `[data.frame]`
 #'
 #' Single data.frame to convert the column data types
 #'

R/mock_listings.R

@@ -7,7 +7,6 @@
 #'
 #' @keywords internal
 #' @family mock apps
-#'
 mock_listings_app <- function(mode = "single") {
   stopifnot(mode %in% c("single", "multi"))
 
@@ -57,9 +56,8 @@ mock_listings_app <- function(mode = "single") {
 #' \code{mock_listings_mm} launches a mock app for the dv.listings shiny module by means of
 #' the module manager (dv.manager).
 #'
+#' @keywords internal
 #' @family mock apps
-#' @export
-#'
 mock_listings_mm <- function() {
   data <- list()
   data[["adsl"]] <- convert_data(pharmaverseadam::adsl)

R/mock_simple_listing.R

@@ -22,7 +22,6 @@ mock_simple_listing_server <- function(input, output, session) {
 }
 
 #' Mock function for simple listing
-#' @export
 mock_simple_listing_mm <- function() {
   stopifnot(
     requireNamespace("dv.manager", quietly = TRUE),

R/mod_listings.R

@@ -71,11 +71,10 @@ listings_UI <- function(module_id) { # nolint
 #' @param pagination `[logical(1) | NULL]` Either a boolean indicating if pagination should be activated, or
 #' NULL for which pagination will be activated for large datasets (nrows > 1000) automatically.
 #'
-#' @param intended_use_label `[character(1) | NULL]` Either a character indicating the intended use for the download, or
-#' NULL. If a label is provided it will be shown before the download and will also be included in the downloaded file.
+#' @param intended_use_label `[character(1) | NULL]` Either a string indicating the intended use for export, or
+#' NULL. The provided label will be displayed prior to the download and will also be included in the exported file.
 #'
 #' @export
-#'
 listings_server <- function(module_id,
                             dataset_list,
                             default_vars = NULL,
@@ -265,15 +264,15 @@ listings_server <- function(module_id,
 }
 
 
-#' Data listings function for module manager
+#' Data listings module for DaVinci's module manager
 #'
 #' @description
 #'
-#' This module will present the dataset in a tabular form using the DT package.
+#' This module will present the dataset as listing using the DT package.
 #' @param dataset_names `[character(1+)]`
 #'
 #' Name(s) of the dataset(s) that will be displayed.
-#' Can not be used together with the parameter \code{dataset_disp}.
+#' Cannot be used together with the parameter \code{dataset_disp}.
 #'
 #' @inheritParams listings_server
 #'
@@ -330,7 +329,6 @@ listings_server <- function(module_id,
 #'   module_list = module_list,
 #'   filter_data = "adsl"
 #' )
-#'
 mod_listings <- function(
     module_id,
     dataset_names,
@@ -347,7 +345,6 @@ mod_listings <- function(
   }
 
   # skip assertions/checks for module_id and default_vars since they will be checked directly in listings_server()
-
   if (!missing(dataset_disp)) {
     checkmate::assert(
       checkmate::check_class(dataset_disp, "mm_dispatcher"),

R/mod_simple_listing.R

@@ -8,8 +8,6 @@
 #' @import shiny
 #'
 #' @name simple_listing
-#'
-
 NULL
 
 #' @describeIn simple_listing UI
@@ -24,8 +22,6 @@ simple_listing_UI <- function(module_id) { # nolint
 #' @describeIn simple_listing server
 #'
 #' @param dataset a data.framish dataset that will be shown as a table
-#'
-#'
 simple_listing_server <- function(module_id, dataset) {
   module <- function(input, output, session) {
     output$table <- DT::renderDataTable({
@@ -42,9 +38,7 @@ utils::globalVariables("filtered_datasets")
 #' @param dataset_disp An mm_dispatch object.
 #' @template module_id-arg
 #'
-#'
 #' @family data_table
-
 mod_simple_listing <- function(dataset_disp, module_id) {
   mod <- list(
     ui = simple_listing_UI,

README.md

@@ -1,40 +1,41 @@
 
 # dv.listings
 
-DaVinci's *dv.listings* module displays arbitrary datasets as listings. 
+The listings module from DaVinci's {dv.listings} package displays arbitrary datasets as listings. 
 Users can select the dataset to be shown, and specify
 which columns of the dataset should be displayed as well as their order.
 The displayed columns can be sorted and filtered. Moreover, the
-table can be filtered to show only entries containing a keyword entered by the user. 
+listing can be filtered to show only entries containing a keyword specified by the user. 
 The module is not limited to usage of one data source,
 most notably, it can handle data from ADaM or SDTM.
 
 <img src="man/figures/table_main_view.PNG" width="100%" />
 
-The module is prepared to be used in combination with *dv.manager* and
+The module is prepared to be used in combination with DaVinci's {dv.manager} package and
 supports its bookmarking functionality.
 
 ## Installation
 
-Feel free to copy the following code chunk to install the latest version of *dv.listings*.
+Feel free to copy the following code chunk to install the latest version of {dv.listings}.
 
 ``` r
 if (!require("remotes")) install.packages("remotes")
 remotes::install_github("Boehringer-Ingelheim/dv.listings")
 ```
 
-Since the *dv.listings* module is intended to be used within an application
-created by means of DaVinci’s *dv.manager* package, make sure that you
-have installed *dv.manager* with a version number equally to or higher than 2.1.0
+Since the listings module is intended to be used within an application
+created by means of DaVinci's module manager, make sure that you
+have installed {dv.manager} with a version number equally to or higher than 2.1.0.
 
 ## Data requirements
 
-As stated above, *dv.listings* can display data from various data sources,
-such as (but not limited to) SDTM and ADaM. However, in order for the
-column filters to work, columns need to be converted to appropriate
-types, e.g. categorial data should be stored as factors, numbers as
-numeric, etc. The easiest way to convert the data is by using
-`convert_data()` which comes along the package. An example can be found
+As stated above, the listings module can display data from various data sources.
+However, for the column filters to function correctly, 
+columns need to be converted to the appropriate data
+types, e.g., categorical data should be stored as factor, numbers as
+numeric, etc. To simplify data conversion, 
+the package provides a convenient function called
+`convert_data()`. An example demonstrating the usage of this function can be found
 in the section below.
 
 Note that `dv.listings` drops row names. In case your dataset is equipped
@@ -49,19 +50,18 @@ attributes(my_data$index)$label <- "Former row names"
 
 ## Example
 
-To define an app containing *dv.listings* using *dv.manager*, you need to
+To define an app based on {dv.listings} and {dv.manager}, you need to
 
 1.  load data,
-2.  make sure that the provided data comply with the requirements of
-    *dv.listings*,
-3.  define a list of modules,
-4.  and launch the app via `run_app()` from *dv.manager*.
+2.  make sure that the provided data comply with the requirements of the listings module,
+3.  define a list of modules that contains the listings module,
+4.  and launch the app via `run_app()` from {dv.manager}.
 
 The following example contains the listed steps. For data protection
-purposes, the example uses dummy data from the *pharmaverseadam* package.
+purposes, the example uses dummy data from {pharmaverseadam}.
 
 ``` r
-library(dv.listing)
+library(dv.listings)
 
 # 1. Create a data list with example data
 data_list <- list(
@@ -105,10 +105,10 @@ dv.manager::run_app(
 ```
 ## Export functionality 
 
-The *dv.listings* module allows users to export listings. Users have the option to either download the currently displayed listing or all available listings.
+The listings module allows users to export listings. Users have the option to either download only the currently displayed listing or all available listings.
 
-For downloading only the currently active listing, the listing will be saved as it is displayed, either in .xlsx or .pdf format. In case any filters have been applied, the downloaded file will reflect this and only include the filtered data.
+To download the currently active listing, it will be saved exactly as it appears on the screen, either in .xlsx or .pdf format. In case any filters have been applied, the downloaded file will reflect these and only include the filtered data.
 
-For users who wish to download all listings, the module allows saving in .xlsx format exclusively. This process disregards any local filters, and each listing is saved in a separate worksheet within the file 
+For users who wish to download all listings, the module allows saving in .xlsx format exclusively. This process disregards any local filters, and each listing is saved in a separate worksheet within the file. 
 
 Please be aware that the PDF download feature is implemented using an RMarkdown file that is rendered into a PDF through LaTeX. As such, it is important to note that a LaTeX installation, along with the necessary packages, is required to use this feature.

_pkgdown.yml

@@ -11,16 +11,10 @@ home:
     href: https://github.com/Boehringer-Ingelheim/dv.listings
 
 reference:
-- title: "Module usage"
+- title: "Modules and utils"
 - contents:
   - mod_listings
-  - mock_listings_mm
   - convert_data
 - title: "For developers"
 - contents:
-  - simple_listing_UI
-  - simple_listing_server
   - mod_simple_listing
-  - listings_UI
-  - listings_server
-  - mock_simple_listing_mm

tests/testthat/apps/simple_listing/app.R

@@ -11,4 +11,4 @@ if (use_load_all) {
   devtools::load_all(pkg_path, quiet = TRUE)
 }
 
-dv.listings::mock_simple_listing_mm()
+dv.listings:::mock_simple_listing_mm()