From 847aa551a0168f43dbaacbfe9cad4f264dcbd20b Mon Sep 17 00:00:00 2001
From: Marco Colombo <marco.colombo@uni-heidelberg.de>
Date: Mon, 9 Sep 2024 10:10:10 +0200
Subject: [PATCH] Remove the defunct na.rm option from plot_GrowthCurve().

---
 NEWS.Rmd                               |  7 ++++--
 R/plot_GrowthCurve.R                   | 35 ++++++++------------------
 man/plot_GrowthCurve.Rd                |  3 ---
 tests/testthat/test_plot_GrowthCurve.R |  6 +----
 4 files changed, 17 insertions(+), 34 deletions(-)

diff --git a/NEWS.Rmd b/NEWS.Rmd
index 993fcc259..53c654330 100644
--- a/NEWS.Rmd
+++ b/NEWS.Rmd
@@ -74,8 +74,11 @@ would produce `Inf` (#203, fixed in #206).
 (#210, fixed in #211).
 
 ### `plot_GrowthCurve()`
-* The function now calculates the relative saturation (`n/N`) using the ration of the two integrates.
-The values is part of the output table.
+* The function now calculates the relative saturation (`n/N`) using the ratio
+of the two integrates. The value is part of the output table.
+* Argument `na.rm` has been removed: as of version 0.9.23, it was defunct
+and only accepted `TRUE` as valid value and produced an error otherwise,
+so there is no effective change in behaviour (#137, fixed in #214).
 
 ### `plot_KDE()`
 * It now officially supports numeric vectors and single-column data frames,
diff --git a/R/plot_GrowthCurve.R b/R/plot_GrowthCurve.R
index af4881214..ec681c6ff 100644
--- a/R/plot_GrowthCurve.R
+++ b/R/plot_GrowthCurve.R
@@ -98,8 +98,6 @@
 #' column name if used. For exponential fits at least three dose points
 #' (including the natural) should be provided.
 #'
-#' @param na.rm [logical] (*with default*): excludes `NA` values from the data set prior to any further operations. This argument is defunct and will be removed in a future version!
-#'
 #' @param mode [character] (*with default*):
 #' selects calculation mode of the function.
 #' - `"interpolation"` (default) calculates the De by interpolation,
@@ -304,7 +302,6 @@
 #' @export
 plot_GrowthCurve <- function(
   sample,
-  na.rm = TRUE,
   mode = "interpolation",
   fit.method = "EXP",
   fit.force_through_origin = FALSE,
@@ -368,29 +365,19 @@ plot_GrowthCurve <- function(
     return(NULL)
   }
 
-  ## optionally, count and exclude NA values and print result
-  if(na.rm[1]) {
-    ## write warning
-    if(sum(!complete.cases(sample)) > 0)
-      warning(paste("[plot_GrowthCurve()]",
-                    sum(!complete.cases(sample)),
-                    "NA value(s) excluded."),
-              call. = FALSE)
-
-    ## exclude NA
-    sample <- na.exclude(sample)
-
-    ##Check if anything is left after removal
-    if(nrow(sample) == 0){
-      message("[plot_GrowthCurve()] Error: After NA removal, nothing is left ",
-              "from the data set, NULL returned")
-      return(NULL)
-    }
+  ## count and exclude NA values and print result
+  if (sum(!complete.cases(sample)) > 0)
+    .throw_warning(sum(!complete.cases(sample)),
+                   " NA values removed")
 
-  }else{
-    stop("[plot_GrowthCurve()] Sorry, the argument 'na.rm' is defunct and will be removed in future!",
-         call. = FALSE)
+  ## exclude NA
+  sample <- na.exclude(sample)
 
+  ## Check if anything is left after removal
+  if (nrow(sample) == 0) {
+    message("[plot_GrowthCurve()] Error: After NA removal, nothing is left ",
+            "from the data set, NULL returned")
+    return(NULL)
   }
 
   ##3. verbose mode
diff --git a/man/plot_GrowthCurve.Rd b/man/plot_GrowthCurve.Rd
index d1449a472..531adfdb1 100644
--- a/man/plot_GrowthCurve.Rd
+++ b/man/plot_GrowthCurve.Rd
@@ -6,7 +6,6 @@
 \usage{
 plot_GrowthCurve(
   sample,
-  na.rm = TRUE,
   mode = "interpolation",
   fit.method = "EXP",
   fit.force_through_origin = FALSE,
@@ -32,8 +31,6 @@ The column for the test dose response is optional, but requires \code{'TnTx'} as
 column name if used. For exponential fits at least three dose points
 (including the natural) should be provided.}
 
-\item{na.rm}{\link{logical} (\emph{with default}): excludes \code{NA} values from the data set prior to any further operations. This argument is defunct and will be removed in a future version!}
-
 \item{mode}{\link{character} (\emph{with default}):
 selects calculation mode of the function.
 \itemize{
diff --git a/tests/testthat/test_plot_GrowthCurve.R b/tests/testthat/test_plot_GrowthCurve.R
index 68bf52be7..6a0266615 100644
--- a/tests/testthat/test_plot_GrowthCurve.R
+++ b/tests/testthat/test_plot_GrowthCurve.R
@@ -93,10 +93,6 @@ test_that("plot_GrowthCurve", {
     "RLum.Results")
   })
 
-  ## test defunct
-  expect_error(
-    object = plot_GrowthCurve(LxTxData[,1:2], output.plot = FALSE, na.rm = FALSE))
-
   ## do not include reg point
   expect_s4_class(
     object = plot_GrowthCurve(
@@ -523,7 +519,7 @@ temp_LambertW <-
     verbose = TRUE),
     "fit.method set to 'LIN'"))
   })
-  expect_match(warnings, "1 NA value(s) excluded",
+  expect_match(warnings, "1 NA values removed",
                all = FALSE, fixed = TRUE)
   expect_match(warnings, "Fitting using an exponential term requires",
                all = FALSE, fixed = TRUE)