Skip to content

Commit

Permalink
stabilize combo of site<- + $<- in degenerate case #163
Browse files Browse the repository at this point in the history
  • Loading branch information
brownag committed Aug 7, 2020
1 parent d5a7d0f commit b80f35c
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 21 deletions.
17 changes: 4 additions & 13 deletions R/SoilProfileCollection-operators.R
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,6 @@ setReplaceMethod("[[", signature(x = "SoilProfileCollection",
if ((!i %in% hznames) &
(i %in% stnames | lv == lx)) {
if (lv == lx | is.null(value)) {
# if(inherits(x@site, "data.table")) {
# x@site[,eval(quote(list(i)))] <- value
# } else {
# x@site[[i]] <- value
# }
s <- data.frame(x@site, stringsAsFactors = FALSE)
s[[i]] <- value
x@site <- .as.data.frame.aqp(s, aqp_df_class(x))
Expand All @@ -393,11 +388,6 @@ setReplaceMethod("[[", signature(x = "SoilProfileCollection",
}
} else if (i %in% hznames | lv == nx) {
if (lv == nx | is.null(value)) {
# if(inherits(x@horizons, "data.table")) {
# x@horizons[, eval(substitute(i))] <- value
# } else {
# x@horizons[[i]] <- value
# }
h <- data.frame(x@horizons, stringsAsFactors = FALSE)
h[[i]] <- value
x@horizons <- .as.data.frame.aqp(h, aqp_df_class(x))
Expand All @@ -406,7 +396,8 @@ setReplaceMethod("[[", signature(x = "SoilProfileCollection",
call. = FALSE)
}
} else {
stop("new data must match either number of profiles or number of horizons",
if(!is.null(value))
stop("new data must match either number of profiles or number of horizons",
call. = FALSE)
}

Expand Down Expand Up @@ -491,7 +482,7 @@ setReplaceMethod("$", signature(x = "SoilProfileCollection"),
}

# working with site data
if(name %in% names(s)) {
if (name %in% names(s)) {
s[[name]] <- value
x@site <- s
return(x)
Expand All @@ -502,7 +493,7 @@ setReplaceMethod("$", signature(x = "SoilProfileCollection"),
n.hz <- nrow(h)
l <- length(value)

if(l == n.hz) {
if (l == n.hz) {
h[[name]] <- value
x@horizons <- h
return(x)
Expand Down
22 changes: 15 additions & 7 deletions R/SoilProfileCollection-setters.R
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ setReplaceMethod("site", signature(object = "SoilProfileCollection"),
ids <- as.character(horizons(object)[[idname(object)]])
ids.coalesce <- .coalesce.idx(ids)

# creation of site data from horizon data
# creation of site data from horizon data
if (inherits(value, "formula")) {
mf <- model.frame(value, object@horizons, na.action = na.pass)
nm <- names(mf)
Expand All @@ -250,6 +250,13 @@ setReplaceMethod("site", signature(object = "SoilProfileCollection"),
ns <- names(value)
nh <- horizonNames(object)

# allow short-circuit
if (all(colnames(value) %in% siteNames(object)) &
nrow(value) == length(object)) {
object@site <- value
return(object)
}

## remove ID column from names(horizons)
ID.idx <- match(idname(object), nh)

Expand Down Expand Up @@ -467,12 +474,13 @@ setReplaceMethod("horizons", signature(object = "SoilProfileCollection"),
if (!inherits(value, "data.frame"))
stop("new horizon data input value must inherit from data.frame", call.=FALSE)

# not required: enforce idname and/or hzidname presence
# idnames <- c(idname(object), hzidname(object))
# if(!all(idnames %in% names(value)))
# stop(sprintf("new horizon data input value should contain column names: %s",
# paste0(idnames, collapse=",")))
#
# allow short circuit
if (all(colnames(value) %in% horizonNames(object)) &
nrow(value) == nrow(object)) {
object@horizons <- value
return(object)
}

# get the corresponding vector of IDs, will be used to compute distinct attributes
ids <- as.character(horizons(object)[[idname(object)]])

Expand Down
22 changes: 21 additions & 1 deletion tests/testthat/test-DT-tbl.R
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,27 @@ res <- lapply(dfclasses, function(use_class) {
expect_equal(horizons(test)[[idname(test)]], as.character(cc(lapply(1:4, rep, 10))))
expect_equal(horizons(test)[[horizonDepths(test)[2]]], cc(rep(1:10, 4)))

# creating and destroying site and horizon column vectors
site(test)$a <- "foo"
site(test)[["b"]] <- "bar"
horizons(test)$c <- "baz"
horizons(test)[["d"]] <- "qux"

expect_equal(test$a, rep("foo", length(test)))
expect_equal(test$b, rep("bar", length(test)))
expect_equal(test$c, rep("baz", nrow(test)))
expect_equal(test$d, rep("qux", nrow(test)))

site(test)$a <- NULL
site(test)[["b"]] <- NULL
test$c <- NULL
horizons(test)$d <- NULL

expect_null(test$a)
expect_null(test$b)
expect_null(test$c)
expect_null(test$d)

# add some horizon data
value <- test_object(data.frame(
id = as.character(2:3),
Expand All @@ -209,7 +230,6 @@ res <- lapply(dfclasses, function(use_class) {
expect_silent({
horizons(test) <- value
}),

# note: data.table sorts correctly without invoking re-order
"data.table" = {
expect_silent( { horizons(test) <- value } )
Expand Down

0 comments on commit b80f35c

Please sign in to comment.