Skip to content

Commit

Permalink
updated tests to avoid lazy evaluation
Browse files Browse the repository at this point in the history
  • Loading branch information
markvanderloo committed Oct 17, 2019
1 parent 416778d commit fbabe62
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 23 deletions.
2 changes: 1 addition & 1 deletion pkg/DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion pkg/NEWS
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/R/stringsim.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}


Expand Down
22 changes: 11 additions & 11 deletions pkg/inst/tinytest/test_seq_dist.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
)

Expand Down
14 changes: 6 additions & 8 deletions pkg/inst/tinytest/test_stringsim.R
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)


Expand Down

0 comments on commit fbabe62

Please sign in to comment.