fix: fix the incorrect handling of the sample parameter in `sample_…

…motifs()` and ensure that the default `sample.size` is integer (#1568)

R/motifs.R

@@ -201,6 +201,7 @@ count_motifs <- function(graph, size = 3, cut.prob = rep(0, size)) {
 #'   of the motif (the `size` argument). By default no cuts are made.
 #' @param sample.size The number of vertices to use as a starting point for
 #'   finding motifs. Only used if the `sample` argument is `NULL`.
+#'   The default is `ceiling(vcount(graph) / 10)` .
 #' @param sample If not `NULL` then it specifies the vertices to use as a
 #'   starting point for finding motifs.
 #' @return A numeric scalar, an estimate for the total number of motifs in
@@ -215,8 +216,13 @@ count_motifs <- function(graph, size = 3, cut.prob = rep(0, size)) {
 #' motifs(g, 3)
 #' count_motifs(g, 3)
 #' sample_motifs(g, 3)
-sample_motifs <- function(graph, size = 3, cut.prob = rep(0, size),
-                          sample.size = vcount(graph) / 10, sample = NULL) {
+sample_motifs <- function(
+  graph,
+  size = 3,
+  cut.prob = rep(0, size),
+  sample.size = NULL,
+  sample = NULL
+) {
   ensure_igraph(graph)
   cut.prob <- as.numeric(cut.prob)
   if (length(cut.prob) != size) {
@@ -226,10 +232,19 @@ sample_motifs <- function(graph, size = 3, cut.prob = rep(0, size),
     )
   }
 
+  if (is.null(sample)) {
+    if (is.null(sample.size)) {
+       sample.size <- ceiling(vcount(graph) / 10)
+    }
+  } else {
+    sample <- as_igraph_vs(graph, sample) - 1
+    sample.size <- 0
+  }
+
   on.exit(.Call(R_igraph_finalizer))
   .Call(
     R_igraph_motifs_randesu_estimate, graph, as.numeric(size),
-    as.numeric(cut.prob), as.numeric(sample.size), as.numeric(sample)
+    as.numeric(cut.prob), as.numeric(sample.size), sample
   )
 }
 

tests/testthat/test-motifs.R

@@ -44,3 +44,19 @@ test_that("motif finding works", {
 
   expect_equal(m5 / m, c(NA, NA, 0.439985332979302, NA, 0.440288166730411, 0.346938775510204, 0.44159753136382, 0.452054794520548, NaN, 0.323076923076923, NaN, 0.347826086956522, NaN, NaN, NaN, NaN))
 })
+
+test_that("sample_motifs works", {
+  withr::local_seed(20041103)
+
+  g <- make_graph(~ A-B-C-A-D-E-F-D-C-F)
+  n <- vcount(g)
+
+  motif_count <- sample_motifs(g)
+  expect_true(0 <= motif_count && motif_count <= n*(n-1)*(n-2) / 6)
+
+  motif_count_letters <- sample_motifs(g, sample = c("C", "D", "E", "F"))
+  expect_true(0 <= motif_count_letters && motif_count_letters <= n*(n-1)*(n-2) / 6)
+
+  motif_count_all <- sample_motifs(g, sample = V(g))
+  expect_true(0 <= motif_count_all && motif_count_all <= n*(n-1)*(n-2) / 6)
+})

tests/testthat/test-plot.R

@@ -74,6 +74,7 @@ test_that("basic plot test, spheres", {
 
 test_that("rglplot() works", {
   skip_if_not_installed("rgl")
+  skip_if(basename(commandArgs())[[1]] == "RDcsan")
 
   # https://stackoverflow.com/a/46320771/5489251
   withr::local_envvar(RGL_USE_NULL = TRUE)