Merge branch '2184_check_keys_renaming' of https://github.com/pharmav…

…erse/admiral into 2184_check_keys_renaming

.Rbuildignore

@@ -29,3 +29,4 @@
 ^staged_dependencies.yaml$
 ^inst/dev_dependencies.R$
 ^\.devcontainer$
+^vignettes/articles$

.github/workflows/common.yml

@@ -69,15 +69,10 @@ jobs:
     uses: pharmaverse/admiralci/.github/workflows/pkgdown.yml@main
     if: github.event_name == 'push' || startsWith(github.ref, 'refs/tags/v')
     with:
-      r-version: "4.1"
-      # Whether to skip multiversion docs
-      # Note that if you have multiple versions of docs,
-      # your URL links are likely to break due to path changes
-      skip-multiversion-docs: false
-      latest-tag-alt-name: cran-release
-      multiversion-docs-landing-page: cran-release
-      branches-or-tags-to-list: >-
-        ^cran-release$|^v([0-9]+\\.)?([0-9]+\\.)?([0-9]+)$
+      r-version: "4.3"
+      skip-multiversion-docs: true
+    secrets:
+      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
   linter:
     name: Lint
     uses: pharmaverse/admiralci/.github/workflows/lintr.yml@main

.github/workflows/templates.yml

@@ -14,4 +14,3 @@ jobs:
     secrets: inherit
     with:
       r-version: "4.1"
-      push-templates-data: true

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: admiral
 Type: Package
 Title: ADaM in R Asset Library
-Version: 0.12.2.9001
+Version: 0.12.3.9002
 Authors@R: c(
     person("Ben", "Straub", email = "[email protected]", role = c("aut", "cre")),
     person("Stefan", "Bundfuss", role = "aut"),
@@ -62,3 +62,4 @@ Suggests:
     tibble
 VignetteBuilder: knitr
 Config/testthat/edition: 3
+Config/Needs/website: gert

NEWS.md

@@ -6,15 +6,21 @@
 
 - `derive_extreme_records()`, `derive_var_extreme_flag()`,`derive_vars_joined()` and `derive_vars_merged()` were enhanced with the arguments `true_value` and `false_value` to align with preexisting functions that had similar functionality (#2125)
 
-- `restrict_derivation()` now allows `{dplyr}` functions like `mutate` in the `derivation argument (#2143)
+- `restrict_derivation()` now allows `{dplyr}` functions like `mutate` in the
+`derivation` argument (#2143)
 
 - `derive_summary_records()`, `derive_var_merged_summary()`, and `get_summary_records()`
 were enhanced such that more than one summary variable can be derived, e.g.,
 `AVAL` as the sum and `ADT` as the maximum of the contributing records. (#1792)
 
+- The `tmp_event_nr_var` argument was added to `derive_extreme_records()` to
+allow more control of the selection of records. It creates a temporary variable
+for the event number, which can be used in `order`. (#2140)
 
 ## Breaking Changes
 
+- `derive_extreme_records()` the `dataset_add` argument is now mandatory. (#2139)
+
 - In `derive_summary_records()` and `get_summary_records()` the arguments
 `analysis_var` and `summary_fun` were deprecated in favor of `set_values_to`.
 (#1792)
@@ -29,6 +35,31 @@ were enhanced such that more than one summary variable can be derived, e.g.,
 - The default value for the `false_value` argument in `derive_extreme_records()` was changed to `NA_character_` (#2125)
 
 - For the function `consolidate_metadata()`, the argument `check_keys` was renamed to `check_type` to align with other functions (#2184)
+- 
+- The `ignore_event_order` argument in `derive_extreme_event()` was deprecated
+and the selection of the records was changed to allow more control. Before, the
+records were selected first by event and then by `order`. Now they are selected
+by `order` only but the event number can be added to it.
+
+   To achieve the old behavior update
+```
+order = exprs(my_order_var),
+ignore_event_order = FALSE,
+```
+to
+```
+tmp_event_nr_var = event_nr,
+order = exprs(event_nr, my_order_var),
+```
+and
+```
+order = exprs(my_order_var),
+ignore_event_order = TRUE,
+```
+to
+```
+order = exprs(my_order_var),
+```
 
 - The following functions, which were deprecated in previous `{admiral}` versions, have been removed: (#2098)
   - `derive_param_extreme_event()`
@@ -61,6 +92,8 @@ were enhanced such that more than one summary variable can be derived, e.g.,
 - Removed Deprecation section in Reference tab.  Added new Superseded section in 
 Reference tab. (#2174)
 
+- Added a link to the previous versions of the website to the navigation bar. (#2205)
+
 ## Various
 
 - Website now has button/links to Slack channel and GitHub Issues (#2127)
@@ -727,7 +760,7 @@ updated to process additional parameter (#1125).
 imputation functions themselves (#1299). I.e., if a derivation like last known alive
 date is based on dates, DTC variables have to be converted to numeric date or
 datetime variables in a preprocessing step. For examples see the [ADSL
-vignette](https://pharmaverse.github.io/admiral/cran-release/articles/adsl.html).
+vignette](https://pharmaverse.github.io/admiral/articles/adsl.html).
   The following arguments were deprecated:
 
   - `date_imputation`, `time_imputation`, and `preserve` in `date_source()`
@@ -826,7 +859,7 @@ empty (#1309)
 
 
 - `create_query_data()` is provided to create the [queries
-dataset](https://pharmaverse.github.io/admiral/cran-release/articles/queries_dataset.html) required as input for `derive_vars_query()` (#606)
+dataset](https://pharmaverse.github.io/admiral/articles/queries_dataset.html) required as input for `derive_vars_query()` (#606)
 
 - `create_single_dose_dataset()` - Derives dataset of single dose from aggregate dose information (#660)
 
@@ -859,7 +892,7 @@ first disease progression. (#1023)
 ### ADLB
 
   - New ADLB template script available `ad_adlb.R`, specific ADLB functions developed and
-  [BDS Finding vignette](https://pharmaverse.github.io/admiral/cran-release/articles/bds_finding.html) has examples enhanced with ADLB functions. (#1122)
+  [BDS Finding vignette](https://pharmaverse.github.io/admiral/articles/bds_finding.html) has examples enhanced with ADLB functions. (#1122)
 
   - `derive_var_shift()` - Derives a character shift variable containing concatenated shift in values based on user-defined pairing (#944)
   - `derive_var_analysis_ratio()` - Derives a ratio variable based on user-supplied variables from a BDS dataset, e.g. ADLB. (#943)
@@ -941,7 +974,7 @@ specific for admiral. Derivations like this can be implemented calling
 - Updated `derive_var_worst_flag()` and `derive_var_extreme_flag()` vignettes to clarify their purpose (#691)
 
 - Added example of ASEQ derivation in ADCM to 
-[OCCDS vignette](https://pharmaverse.github.io/admiral/cran-release/articles/occds.html#aseq)
+[OCCDS vignette](https://pharmaverse.github.io/admiral/articles/occds.html#aseq)
 (#720)
 
 - Examples have been added for `format_reason_default()`, `format_eoxxstt_default()`, `extend_source_datasets()` and `filter_date_sources()` (#745)
@@ -1041,7 +1074,7 @@ this case the day is imputed as `15` (#592)
 
 - The first truly open source release licensed under Apache 2.0 (#680)
 
-- New vignette [Contributing to admiral](https://pharmaverse.github.io/admiral/cran-release/articles/contribution_model.html) (#679)
+- New vignette [Contributing to admiral](https://pharmaverse.github.io/admiral/articles/contribution_model.html) (#679)
 
 - New vignette [Unit Test Guidance](https://pharmaverse.github.io/admiraldev/main/articles/unit_test_guidance.html) (#679)
 
@@ -1063,7 +1096,7 @@ age in different units (#569)
 - `derive_param_tte()` derives time-to-event-parameters (#546)
 
 - For common time-to-event endpoints [event and censoring source
-objects](https://pharmaverse.github.io/admiral/cran-release/reference/index.html#section-pre-defined-time-to-event-sources) are
+objects](https://pharmaverse.github.io/admiral/reference/index.html#section-pre-defined-time-to-event-sources) are
 provided (#612)
 
 ### Developer
@@ -1098,9 +1131,9 @@ to specify the unit of the input age (#569)
 
 ## Documentation
 
-- New vignette [Creating a BDS Time-to-Event ADaM](https://pharmaverse.github.io/admiral/cran-release/articles/bds_tte.html) (#549)
+- New vignette [Creating a BDS Time-to-Event ADaM](https://pharmaverse.github.io/admiral/articles/bds_tte.html) (#549)
 
-- New vignette [Queries Dataset Documentation](https://pharmaverse.github.io/admiral/cran-release/articles/queries_dataset.html) (#561)
+- New vignette [Queries Dataset Documentation](https://pharmaverse.github.io/admiral/articles/queries_dataset.html) (#561)
 
 - New vignette [Writing Vignettes](https://pharmaverse.github.io/admiraldev/main/articles/writing_vignettes.html) (#334)
 
@@ -1295,10 +1328,10 @@ to specify the unit of the input age (#569)
 
 ## Documentation
 
-- [Frequently Asked Questions](https://pharmaverse.github.io/admiral/cran-release/articles/faq.html)
+- [Frequently Asked Questions](https://pharmaverse.github.io/admiral/articles/faq.html)
 
-- [Creating ADSL](https://pharmaverse.github.io/admiral/cran-release/articles/adsl.html)
+- [Creating ADSL](https://pharmaverse.github.io/admiral/articles/adsl.html)
 
-- [Creating a BDS Finding ADaM](https://pharmaverse.github.io/admiral/cran-release/articles/bds_finding.html)
+- [Creating a BDS Finding ADaM](https://pharmaverse.github.io/admiral/articles/bds_finding.html)
 
-- [Creating an OCCDS ADaM](https://pharmaverse.github.io/admiral/cran-release/articles/occds.html)
+- [Creating an OCCDS ADaM](https://pharmaverse.github.io/admiral/articles/occds.html)

R/derive_extreme_event.R

@@ -18,6 +18,16 @@
 #'   For `event_joined()` events the observations are selected by calling
 #'   `filter_joined`. The `condition` field is passed to the `filter` argument.
 #'
+#' @param tmp_event_nr_var Temporary event number variable
+#'
+#'   The specified variable is added to all source datasets and is set to the
+#'   number of the event before selecting the records of the event.
+#'
+#'   It can be used in `order` to determine which record should be used if
+#'   records from more than one event are selected.
+#'
+#'   The variable is not included in the output dataset.
+#'
 #' @param order Sort order
 #'
 #'   If a particular event from `events` has more than one observation, within
@@ -41,6 +51,13 @@
 #'
 #' @param ignore_event_order Ignore event order
 #'
+#'   `r lifecycle::badge("deprecated")`
+#'
+#'   This argument is *deprecated*. If event order should be ignored, please
+#'   specify neither `ignore_event_order` nor `tmp_event_nr_var`. If the event
+#'   order should be considered, don't specify `ignore_event_order` but specify
+#'   `tmp_event_nr_var` and and the specified variable to `order`.
+#'
 #'   If the argument is set to `TRUE`, all events defined by `events` are
 #'   considered equivalent. If there is more than one observation per by group
 #'   the first or last (with respect to `mode` and `order`) is select without
@@ -73,12 +90,14 @@
 #'
 #'       1. The variables specified by the `set_values_to` field of the event
 #'       are added to the selected observations.
+#'       1. The variable specified for `tmp_event_nr_var` is added and set to
+#'       the number of the event.
 #'       1. Only the variables specified for the `keep_source_vars` field of the
 #'       event, and the by variables (`by_vars`) and the variables created by
 #'       `set_values_to` are kept.
+#'   1. All selected observations are bound together.
 #'   1. For each group (with respect to the variables specified for the
-#'   `by_vars` parameter) the first event is selected. If there is more than one
-#'   observation per event the first or last observation (with respect to the
+#'   `by_vars` parameter) the first or last observation (with respect to the
 #'   order specified for the `order` parameter and the mode specified for the
 #'   `mode` parameter) is selected.
 #'   1. The variables specified by the `set_values_to` parameter are added to
@@ -141,8 +160,9 @@
 #'       set_values_to = exprs(AVALC = "Missing", AVAL = 99)
 #'     )
 #'   ),
-#'   order = exprs(ADY),
-#'   mode = "last",
+#'   tmp_event_nr_var = event_nr,
+#'   order = exprs(event_nr, desc(ADY)),
+#'   mode = "first",
 #'   set_values_to = exprs(
 #'     PARAMCD = "WSP",
 #'     PARAM = "Worst Sleeping Problems"
@@ -178,7 +198,8 @@
 #'       set_values_to = exprs(AVALC = "Y")
 #'     )
 #'   ),
-#'   order = exprs(AVISITN),
+#'   tmp_event_nr_var = event_nr,
+#'   order = exprs(event_nr, AVISITN),
 #'   mode = "first",
 #'   keep_source_vars = exprs(AVISITN),
 #'   set_values_to = exprs(
@@ -243,7 +264,8 @@
 #' derive_extreme_event(
 #'   adrs,
 #'   by_vars = exprs(USUBJID),
-#'   order = exprs(ADT),
+#'   tmp_event_nr_var = event_nr,
+#'   order = exprs(event_nr, ADT),
 #'   mode = "first",
 #'   source_datasets = list(adsl = adsl),
 #'   events = list(
@@ -321,6 +343,7 @@
 derive_extreme_event <- function(dataset,
                                  by_vars = NULL,
                                  events,
+                                 tmp_event_nr_var = NULL,
                                  order,
                                  mode,
                                  source_datasets = NULL,
@@ -355,7 +378,29 @@ derive_extreme_event <- function(dataset,
     )
   }
 
-  assert_logical_scalar(ignore_event_order)
+  if (!missing(ignore_event_order)) {
+    assert_logical_scalar(ignore_event_order)
+    if (ignore_event_order) {
+      deprecate_details <- paste(
+        "The event order is ignored by default.",
+        "Please remove `ignore_event_order = TRUE` from the call.",
+        sep = "\n"
+      )
+    } else {
+      deprecate_details <- c(
+        "Please remove `ignore_event_order = FALSE` from the call.",
+        "Specify `tmp_event_nr_var`.",
+        "Add the specified variable to `order`."
+      )
+    }
+    deprecate_warn(
+      "1.0.0",
+      "derive_extreme_event(ignore_event_order=)",
+      "derive_extreme_event(tmp_event_nr_var=)",
+      details = deprecate_details
+    )
+  }
+  tmp_event_nr_var <- assert_symbol(enexpr(tmp_event_nr_var), optional = TRUE)
   check_type <-
     assert_character_scalar(
       check_type,
@@ -368,11 +413,6 @@ derive_extreme_event <- function(dataset,
   # Create new observations
   ## Create a dataset (selected_records) from `events`
   event_index <- as.list(seq_along(events))
-  if (ignore_event_order) {
-    tmp_event_no <- NULL
-  } else {
-    tmp_event_no <- get_new_tmp_var(dataset, prefix = "tmp_event_no")
-  }
 
   selected_records_ls <- map2(
     events,
@@ -383,6 +423,9 @@ derive_extreme_event <- function(dataset,
       } else {
         data_source <- source_datasets[[event$dataset_name]]
       }
+      if (!is.null(tmp_event_nr_var)) {
+        data_source <- mutate(data_source, !!tmp_event_nr_var := index)
+      }
       if (is.null(event$order)) {
         event_order <- order
       } else {
@@ -417,41 +460,26 @@ derive_extreme_event <- function(dataset,
       } else {
         event_keep_source_vars <- event$keep_source_vars
       }
-      if (!ignore_event_order) {
-        data_events <- mutate(data_events, !!tmp_event_no := index)
-      }
       data_events %>%
         process_set_values_to(set_values_to = event$set_values_to) %>%
-        select(!!!event_keep_source_vars, !!tmp_event_no, !!!by_vars, names(event$set_values_to))
+        select(
+          !!!event_keep_source_vars, !!tmp_event_nr_var, !!!by_vars,
+          names(event$set_values_to)
+        )
     }
   )
   selected_records <- bind_rows(selected_records_ls)
 
-  ## tmp obs number within by_vars and a type of event
-  tmp_obs <- get_new_tmp_var(selected_records)
-  selected_records <- selected_records %>%
-    derive_var_obs_number(
-      new_var = !!tmp_obs,
-      order = order,
-      by_vars = expr_c(by_vars, tmp_event_no),
-      check_type = check_type
-    )
-
   ## filter_extreme
-  if (mode == "first") {
-    tmp_obs_expr <- expr(!!tmp_obs)
-  } else {
-    tmp_obs_expr <- expr(desc(!!tmp_obs))
-  }
   new_obs <- selected_records %>%
     filter_extreme(
       by_vars = by_vars,
-      order = expr_c(tmp_event_no, tmp_obs_expr),
-      mode = "first",
+      order = order,
+      mode = mode,
       check_type = check_type
     ) %>%
     mutate(!!!set_values_to) %>%
-    select(-!!tmp_event_no, -!!tmp_obs)
+    select(-!!tmp_event_nr_var)
 
   # Create output dataset
   bind_rows(dataset, new_obs)