From 711563ff5939687f33f79afe1e13e5c08e2e822c Mon Sep 17 00:00:00 2001
From: Ben Bond-Lamberty <bondlamberty@pnnl.gov>
Date: Sat, 17 Feb 2024 09:02:48 -0500
Subject: [PATCH] whattheflux.quiet=TRUE should not silence warnings (#24)

* Check for missing metadata dates and times
* whattheflux.quiet=TRUE should not silence warnings
---
 R/metadata.R                   | 11 ++++++++++-
 R/models.R                     |  6 +++---
 R/zzz.R                        |  7 -------
 tests/testthat/test-metadata.R | 10 ++++++++++
 tests/testthat/test-zzz.R      |  2 --
 5 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/R/metadata.R b/R/metadata.R
index 3adbcae..2c7099c 100644
--- a/R/metadata.R
+++ b/R/metadata.R
@@ -92,6 +92,15 @@ wtf_metadata_match <- function(data_timestamps,
   stopifnot(length(start_dates) == length(dead_bands))
   stopifnot(length(start_dates) == length(obs_lengths))
 
+  # The metadata dates and times shouldn't be empty
+  if(any(is.na(start_dates))) {
+    warning("One or more metadata dates are missing")
+  }
+  if(any(is.na(start_times))) {
+    warning("One or more metadata times are missing")
+  }
+
+  # Convert things to POSIXct and check validity
   if(is.character(data_timestamps)) data_timestamps <- ymd_hms(data_timestamps)
   stopifnot(is.POSIXct(data_timestamps))
 
@@ -114,7 +123,7 @@ wtf_metadata_match <- function(data_timestamps,
   # single machine that generated the data
   ord <- order(start_timestamps)
   overlaps <- head(stop_timestamps[ord], -1) >= tail(start_timestamps[ord], -1)
-  if(any(overlaps)) {
+  if(any(overlaps, na.rm = TRUE)) {
     stop("start_timestamps overlaps: ",
          paste(which(overlaps) + 1, collapse = ", "))
   }
diff --git a/R/models.R b/R/models.R
index 1448cc4..7ac0e0a 100644
--- a/R/models.R
+++ b/R/models.R
@@ -28,7 +28,7 @@ wtf_fit_models <- function(time, conc) {
   # Basic linear model
   try(mod <- lm(conc ~ time))
   if(!exists("mod")) {
-    wtf_warning("Could not fit linear model")
+    warning("Could not fit linear model")
     return(NULL)
   }
 
@@ -48,7 +48,7 @@ wtf_fit_models <- function(time, conc) {
     coefficients(robust)[2]
   },
   error = function(e) {
-    wtf_warning("Could not fit robust linear model")
+    warning("Could not fit robust linear model")
     NA_real_
   })
 
@@ -58,7 +58,7 @@ wtf_fit_models <- function(time, conc) {
     summary(poly)$r.squared
   },
   error = function(e) {
-    wtf_warning("Could not fit polynomial model")
+    warning("Could not fit polynomial model")
     NA_real_
   })
 
diff --git a/R/zzz.R b/R/zzz.R
index 8afb53a..3fc5fb1 100644
--- a/R/zzz.R
+++ b/R/zzz.R
@@ -6,10 +6,3 @@ wtf_message <- function(...) {
   }
   message(...)
 }
-
-wtf_warning <- function(...) {
-  if (getOption("whattheflux.quiet", default = FALSE)) {
-    return()
-  }
-  warning(...)
-}
diff --git a/tests/testthat/test-metadata.R b/tests/testthat/test-metadata.R
index f03e681..259b036 100644
--- a/tests/testthat/test-metadata.R
+++ b/tests/testthat/test-metadata.R
@@ -85,6 +85,16 @@ test_that("wtf_metadata_match works", {
   expect_silent(x <- wtf_metadata_match(d_t, s_d, s_t, db, ol))
   expect_identical(x, c(1, 1, 2, NA_real_))
 
+  # Warns on missing metadata dates
+  suppressMessages({
+    s_d[1] <- NA
+    expect_warning(wtf_metadata_match(d_t, s_d, s_t, db, ol),
+                   regexp = "dates are missing")
+  })
+  # We also warn on missing times, but checking that requires a nested
+  # expect_warning here, because of hms() behavior, which crashes
+  # on GitHub Actions
+
   # Gives a message if there are unmatched metadata entries
   s_d <- c("2024-01-01", "2024-01-01", "2024-01-10")
   s_t <- c("13:00:00", "13:05:00", "13:10:00")
diff --git a/tests/testthat/test-zzz.R b/tests/testthat/test-zzz.R
index e92f121..9f8a7b9 100644
--- a/tests/testthat/test-zzz.R
+++ b/tests/testthat/test-zzz.R
@@ -2,9 +2,7 @@ test_that("quiet functionality works", {
   # Check that the wtf_message and wtf_warning functions respect quiet option
   withr::local_options(whattheflux.quiet = TRUE)
   expect_no_message(wtf_message("hi"))
-  expect_no_warning(wtf_warning("hi"))
 
   withr::local_options(whattheflux.quiet = FALSE)
   expect_message(wtf_message("hi"), regexp = "hi")
-  expect_warning(wtf_warning("hi"), regexp = "hi")
 })