From fbabe622ceafb52997ad6951a4526d47b4aa7068 Mon Sep 17 00:00:00 2001 From: Mark van der Loo Date: Thu, 17 Oct 2019 17:45:58 +0200 Subject: [PATCH] updated tests to avoid lazy evaluation --- pkg/DESCRIPTION | 2 +- pkg/NEWS | 8 +++++++- pkg/R/stringsim.R | 4 ++-- pkg/inst/tinytest/test_seq_dist.R | 22 +++++++++++----------- pkg/inst/tinytest/test_stringsim.R | 14 ++++++-------- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/pkg/DESCRIPTION b/pkg/DESCRIPTION index b073a04..c07a111 100644 --- a/pkg/DESCRIPTION +++ b/pkg/DESCRIPTION @@ -18,7 +18,7 @@ Description: Implements an approximate string matching version of R's native character vectors while taking proper care of encoding or between integer vectors representing generic sequences. This package is built for speed and runs in parallel by using 'openMP'. An API for C or C++ is exposed as well. -Version: 0.9.5.3 +Version: 0.9.5.4 Depends: R (>= 2.15.3) URL: https://github.com/markvanderloo/stringdist diff --git a/pkg/NEWS b/pkg/NEWS index bce7fec..dc251ae 100644 --- a/pkg/NEWS +++ b/pkg/NEWS @@ -1,4 +1,10 @@ -versioni 0.9.5.3 +version 0.9.5.4 +- Some tests using seq_dist() would fail when the input was defined with lazily + evaluated arguments, e.g. list(1:3, 2:4); but only in the context of NSE by a + test suite ('tinytest', 'testthat'). Tests were replaced by literal versions, + e.g. list(c(1,2,3), c(2,3,4)). + +version 0.9.5.3 - Update in test suite to stay on CRAN version 0.9.5.2 diff --git a/pkg/R/stringsim.R b/pkg/R/stringsim.R index 73446f9..8638c90 100644 --- a/pkg/R/stringsim.R +++ b/pkg/R/stringsim.R @@ -67,10 +67,10 @@ stringsim <- function(a, b, method = c("osa", "lv", "dl", "hamming", "lcs", #' @export seq_sim <- function(a, b, method = c("osa", "lv", "dl", "hamming", "lcs", "qgram", "cosine", "jaccard", "jw"), q = 1, ...) { - + method <- match.arg(method) dist <- stringdist::seq_dist(a, b, method=method, q=q, ...) - normalize_dist(dist,a,b,method=method,q=q) + normalize_dist(dist, a, b, method=method, q=q) } diff --git a/pkg/inst/tinytest/test_seq_dist.R b/pkg/inst/tinytest/test_seq_dist.R index f64de94..9a2eb09 100644 --- a/pkg/inst/tinytest/test_seq_dist.R +++ b/pkg/inst/tinytest/test_seq_dist.R @@ -34,12 +34,12 @@ options(sd_num_thread=2) ## Conversion for non-integer-list arguments - expect_equal(seq_dist(list(1:3),list(2:4)),seq_dist(as.numeric(1:3),as.numeric(2:4))) - expect_equal(seq_dist(list(1:3),list(2:4)),seq_dist(1:3, 2:4)) - expect_equal(seq_distmatrix(list(1:3),list(2:4)), seq_distmatrix(as.numeric(1:3),as.numeric(2:4))) - expect_equal(seq_distmatrix(list(1:3),list(2:4)), seq_distmatrix(1:3,2:4)) - expect_equal(seq_distmatrix(list(1:3)),seq_distmatrix(1:3)) - expect_equal(seq_distmatrix(list(1:3)),seq_distmatrix(as.numeric(1:3))) + expect_equal(seq_dist(list(c(1,2,3)),list(c(2,3,4))),seq_dist(as.numeric(c(1,2,3)),as.numeric(c(2,3,4)))) + expect_equal(seq_dist(list(c(1,2,3)),list(c(2,3,4))),seq_dist(c(1,2,3), c(2,3,4))) + expect_equal(seq_distmatrix(list(c(1,2,3)),list(c(2,3,4))), seq_distmatrix(as.numeric(c(1,2,3)),as.numeric(c(2,3,4)))) + expect_equal(seq_distmatrix(list(c(1,2,3)),list(c(2,3,4))), seq_distmatrix(c(1,2,3),c(2,3,4))) + expect_equal(seq_distmatrix(list(c(1,2,3))),seq_distmatrix(c(1,2,3))) + expect_equal(seq_distmatrix(list(c(1,2,3))),seq_distmatrix(as.numeric(c(1,2,3)))) ## Some edge cases @@ -52,20 +52,20 @@ options(sd_num_thread=2) expect_equivalent(seq_distmatrix(1:10),dist(0)) expect_equivalent(seq_distmatrix(1:10,list(1:10)),matrix(0)) expect_equivalent( - as.matrix(seq_distmatrix(list(1:3,2:4)) ) + as.matrix(seq_distmatrix(list(c(1,2,3),c(2,3,4))) ) , matrix(c(0,2,2,0),nrow=2) ) expect_equal( - as.matrix(seq_distmatrix(list(x=1:3,y=2:4),useNames="names") ) + as.matrix(seq_distmatrix(list(x=c(1,2,3),y=c(2,3,4)),useNames="names") ) , matrix(c(0,2,2,0),nrow=2,dimnames=list(c('x','y'),c('x','y'))) ) expect_equal( - seq_distmatrix(list(x=1:3,y=2:4),list(x=1:3,y=2:4),useNames="names") + seq_distmatrix(list(x=c(1,2,3),y=c(2,3,4)),list(x=c(1,2,3),y=c(2,3,4)),useNames="names") , matrix(c(0,2,2,0),nrow=2,dimnames=list(c('x','y'),c('x','y'))) ) - expect_equal(class(seq_distmatrix(list(1:3,2:4))),"dist") + expect_equal(class(seq_distmatrix(list(c(1,2,3),c(2,3,4)))),"dist") expect_equivalent( - as.matrix(seq_distmatrix(list(1:3,2:4)),seq_distmatrix(list(1:3,2:4),list(1:3,2:4)) ) + as.matrix(seq_distmatrix(list(c(1,2,3),c(2,3,4))),seq_distmatrix(list(c(1,2,3),c(2,3,4)),list(c(1,2,3),c(2,3,4))) ) , matrix(c(0,2,2,0),nrow=2) ) diff --git a/pkg/inst/tinytest/test_stringsim.R b/pkg/inst/tinytest/test_stringsim.R index 79a513a..249e5f8 100644 --- a/pkg/inst/tinytest/test_stringsim.R +++ b/pkg/inst/tinytest/test_stringsim.R @@ -52,15 +52,13 @@ for (method in methods[6:8]){ ## seq_sim -## elementary seq_sim test -# -# There seams to be an infrequently occurring edge case -# when 2 threads are used. We turn it of for now so we can -# keep stringdist on CRAN. -options(sd_num_thread=1) +# We used to have list(1:3, 2:4) and list(1:3). This occasionally +# gave failing tests, and only in the context of expect_equal (both +# for tinytest and testthat. Therefore this may point to a hard-to-reproduce +# bug in R's JIT compiler. expect_equal( - seq_sim(list(1:3,2:4),list(1:3)) - , stringsim(c("abc","bcd"),"abc") + seq_sim(list(c(1,2,3),c(2,3,4)), list(c(1,2,3)), method="cosine") + , stringsim(c("abc","bcd"),"abc", method="cosine") )