Merge pull request #310 from ncss-tech/hzOrder

Remove `"original.order"` from SPC metadata
ncss-tech · Mar 20, 2024 · 175d170 · 175d170
2 parents 7e5e290 + e3a8f5b
commit 175d170
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 16 deletions.
diff --git a/R/Class-SoilProfileCollection.R b/R/Class-SoilProfileCollection.R
@@ -200,16 +200,9 @@ setClass(
       aqp_hzdesgn = "",
       aqp_hztexcl = "",
       depth_units = 'cm',
-      stringsAsFactors = FALSE,
-
-      # calculate data order (original)
-      original.order = order(as.character(horizons[[idcol]]),
-                             horizons[[depthcols[1]]])
+      stringsAsFactors = FALSE
     )
 
-    # the target order to check/maintain is the default for a new SPC
-    # metadata$target.order <- metadata$original.order
-
     # add any custom metadata
     metadata <- c(metadata,
                   new.metadata[!names(new.metadata) %in% names(metadata)])

diff --git a/R/SoilProfileCollection-integrity.R b/R/SoilProfileCollection-integrity.R
@@ -94,10 +94,7 @@ setMethod('reorderHorizons',
 
             h <- object@horizons
 
-            if (is.null(target.order))
-              target.order <- metadata(object)$original.order
-              if (is.null(target.order))
-                target.order <- 1:nrow(h)
+            stopifnot(!is.null(target.order))
 
             current.order <- match(target.order,
                                    order(as.character(h[[idname(object)]]),

diff --git a/R/SoilProfileCollection-metadata.R b/R/SoilProfileCollection-metadata.R
@@ -73,7 +73,9 @@
   customattr <- customattr[!names(customattr) %in% names(attributes(SoilProfileCollection()))]
   attributes(dest)[names(customattr)] <- attributes(src)[names(customattr)]
 
+  # original.order metadata no longer created, not transferred
   cols <- names(m)[names(m) != "original.order"]
+
   metadata(dest)[cols] <- m[cols]
   dest
 }

diff --git a/tests/testthat/test-SPC-objects.R b/tests/testthat/test-SPC-objects.R
@@ -611,15 +611,15 @@ test_that("basic integrity checks", {
   expect_true(spc_in_sync(spc[0,])$valid)
 
   # reordering the horizons with reorderHorizons resolves integrity issues
-  expect_true(spc_in_sync(reorderHorizons(spc))$valid)
+  expect_true(spc_in_sync(reorderHorizons(spc, seq(nrow(spc))))$valid)
 
-  # default reordering uses metadata$original.order, here we override and reverse it
-  expect_false(spc_in_sync(reorderHorizons(spc, rev(spc@metadata$original.order)))$valid)
+  # override and reverse it
+  expect_false(spc_in_sync(reorderHorizons(spc, rev(seq(nrow(spc)))))$valid)
 
   # removing the metadata works because target order matches sequential order
   #  this cannot be guaranteed to be the case in general but is a reasonable default
   spc@metadata$target.order <- NULL
-  expect_true(spc_in_sync(reorderHorizons(spc))$valid)
+  expect_true(spc_in_sync(reorderHorizons(spc, seq(nrow(spc))))$valid)
 
   # reordering horizons with any order works, even if invalid
   spc <- reorderHorizons(spc, target.order = c(20:40,1:19))