Merge pull request #4000 from anshul23102/fix/covariate-functions-null-return

fix(db): return data unchanged instead of NULL when no covariates found

Author: dlebauer

base/db/NEWS.md

@@ -2,6 +2,8 @@
 
 ## Fixed
 
+* `arrhenius.scaling.traits()`: previously returned `NULL` when no temperature covariates were found, crashing `query.trait.data()` with `argument is of length zero`. The function now drops rows that lack a measurement temperature covariate and emits a `logger.warn()` with the row count. If no observations have any temperature covariate, an empty data frame (zero rows, same columns) is returned. The `missing.temp` argument is retained for backward compatibility but is no longer applied.
+* `filter_sunleaf_traits()`: returned `NULL` instead of `data` unchanged when no `canopy_layer` covariate was found. Now returns the input data frame unmodified in that case, consistent with the more standardised measurement protocol for sun-leaf traits.
 * `query.trait.data()`: the `warning()` call for missing trait data was placed after `return(NA)` and therefore never fired. Moved before the return and changed to `logger.warn()` for consistency with the rest of the codebase.
 
 * Refactored `convert.input()` internals into smaller, and hopefully more testable, chunks. No user-visible changes expected.

base/db/R/covariate.functions.R

@@ -53,37 +53,57 @@ query.covariates <- function(trait.ids, con = NULL, ...){
 ##--------------------------------------------------------------------------------------------------#
 ##' Apply Arrhenius scaling to 25 degC for temperature-dependent traits
 ##'
+##' Rows whose measurement temperature covariate is missing are dropped with a
+##' warning rather than silently assigned a default temperature.  If no
+##' temperature covariate is recorded for any observation the function returns
+##' an empty data frame (zero rows, same columns as \code{data}).
+##'
 ##' @param data data frame of data to scale, as returned by query.data()
 ##' @param covariates data frame of covariates, as returned by query.covariates().
-##'   Note that data with no matching covariates will be unchanged.
 ##' @param temp.covariates names of covariates used to adjust for temperature;
 ##'   if length > 1, order matters (first will be used preferentially)
-##' @param new.temp the reference temperature for the scaled traits. Curerntly 25 degC
-##' @param missing.temp the temperature assumed for traits with no covariate found. Curerntly 25 degC
+##' @param new.temp the reference temperature for the scaled traits. Currently 25 degC
+##' @param missing.temp no longer used; kept for backward compatibility only
 ##' @author Carl Davidson, David LeBauer, Ryan Kelly
 arrhenius.scaling.traits <- function(data, covariates, temp.covariates, new.temp = 25, missing.temp = 25){
   # Select covariates that match temp.covariates
-  covariates <- covariates[covariates$name %in% temp.covariates,]
-  
-  if(nrow(covariates)>0) {
+  covariates <- covariates[covariates$name %in% temp.covariates, ]
+
+  if (nrow(covariates) > 0) {
     # Sort covariates in order of priority
     covariates <- do.call(rbind,
-                          lapply(temp.covariates, function(temp.covariate) covariates[covariates$name == temp.covariate, ])
+                          lapply(temp.covariates, function(tc) covariates[covariates$name == tc, ])
     )
-    
+
     data <- append.covariate(data, 'temp', covariates)
-    
-    # Assign default value for traits with no covariates
-    data$temp[is.na(data$temp)] <- missing.temp
-    
+
+    # Drop rows that have no temperature covariate recorded
+    n_missing <- sum(is.na(data$temp))
+    if (n_missing > 0) {
+      PEcAn.logger::logger.warn(
+        n_missing, "row(s) of trait data dropped due to missing temperature covariate."
+      )
+      data <- data[!is.na(data$temp), ]
+    }
+
+    # Remove temporary covariate column before returning if nothing survived
+    if (nrow(data) == 0) {
+      return(data[, colnames(data) != 'temp', drop = FALSE])
+    }
+
     # Scale traits
-    data$mean <- PEcAn.utils::arrhenius.scaling(observed.value = data$mean, old.temp = data$temp, new.temp=new.temp)
-    data$stat <- PEcAn.utils::arrhenius.scaling(observed.value = data$stat, old.temp = data$temp, new.temp=new.temp)
-    
-    #remove temporary covariate column.
-    data<-data[,colnames(data)!='temp']
+    data$mean <- PEcAn.utils::arrhenius.scaling(observed.value = data$mean, old.temp = data$temp, new.temp = new.temp)
+    data$stat  <- PEcAn.utils::arrhenius.scaling(observed.value = data$stat,  old.temp = data$temp, new.temp = new.temp)
+
+    # Remove temporary covariate column
+    data <- data[, colnames(data) != 'temp', drop = FALSE]
   } else {
-    data <- NULL
+    # No temperature covariates found for any observation; drop all rows.
+    n_rows <- nrow(data)
+    PEcAn.logger::logger.warn(
+      n_rows, "row(s) of trait data dropped: no temperature covariate found for any observation."
+    )
+    data <- data[0, , drop = FALSE]
   }
   return(data)
 }
@@ -108,7 +128,7 @@ filter_sunleaf_traits <- function(data, covariates){
     # remove temporary covariate column
     data <- data[,colnames(data)!='canopy_layer']
   } else {
-    data <- NULL
+    # No canopy_layer covariate found; return data unchanged rather than NULL.
   }
   return(data)
 }

base/db/man/arrhenius.scaling.traits.Rd

@@ -1,3 +1,73 @@
+## Helper: minimal trait data frame used across several tests
+make_trait_data <- function(ids = 1:4) {
+  data.frame(
+    id   = ids,
+    mean = c(10, 20, 30, 40)[seq_along(ids)],
+    stat = c(1, 2, 3, 4)[seq_along(ids)],
+    n    = rep(5L, length(ids)),
+    stringsAsFactors = FALSE
+  )
+}
+
+## Helper: minimal covariates data frame
+make_temp_covariates <- function(trait_ids, temps) {
+  data.frame(
+    trait_id = trait_ids,
+    level    = temps,
+    name     = rep("Tleaf", length(trait_ids)),
+    stringsAsFactors = FALSE
+  )
+}
+
+test_that("arrhenius.scaling.traits scales all rows when every row has a covariate", {
+  data       <- make_trait_data(1:3)
+  covariates <- make_temp_covariates(1:3, c(20, 25, 30))
+
+  result <- arrhenius.scaling.traits(data, covariates, temp.covariates = "Tleaf")
+
+  expect_equal(nrow(result), 3)
+  expect_false("temp" %in% colnames(result))
+  # Row measured at 25 degC should be unchanged after scaling to 25 degC
+  expect_equal(result$mean[2], data$mean[2])
+})
+
+test_that("arrhenius.scaling.traits drops rows with missing covariate and warns", {
+  data       <- make_trait_data(1:4)
+  # Only rows 1 and 3 have covariates
+  covariates <- make_temp_covariates(c(1, 3), c(20, 30))
+
+  expect_warning(
+    result <- arrhenius.scaling.traits(data, covariates, temp.covariates = "Tleaf"),
+    regexp = NA  # warning comes via logger.warn, not warning(); just confirm no error
+  )
+  # Rows 2 and 4 (no covariate) must be dropped
+  expect_equal(nrow(result), 2)
+  expect_true(all(result$id %in% c(1, 3)))
+  expect_false("temp" %in% colnames(result))
+})
+
+test_that("arrhenius.scaling.traits returns empty data frame when no covariates at all", {
+  data       <- make_trait_data(1:3)
+  covariates <- data.frame(trait_id = integer(0), level = numeric(0),
+                           name = character(0), stringsAsFactors = FALSE)
+
+  result <- arrhenius.scaling.traits(data, covariates, temp.covariates = "Tleaf")
+
+  expect_equal(nrow(result), 0)
+  # Columns should be preserved
+  expect_true(all(c("id", "mean", "stat", "n") %in% colnames(result)))
+})
+
+test_that("filter_sunleaf_traits returns data unchanged when covariates is empty", {
+  data       <- make_trait_data(1:3)
+  covariates <- data.frame(trait_id = integer(0), level = numeric(0),
+                           name = character(0), stringsAsFactors = FALSE)
+
+  result <- filter_sunleaf_traits(data, covariates)
+
+  expect_equal(result, data)
+})
+
 test_that("`append.covariate` able to append new column for covariates in given data based on id", {
   data <- data.frame(
     id = c(1, 2, 3, 4),