From 232629d4a0bf0960e53c6f2b10eb25759f83ec53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20Horva=CC=81t?= Date: Thu, 31 Oct 2024 22:34:04 +0000 Subject: [PATCH 1/5] fix: fix the incorrect handling of the `sample` parameter in `sample_motifs()` and ensure that the default `sample.size` is integer --- R/motifs.R | 8 ++++++-- man/sample_motifs.Rd | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/R/motifs.R b/R/motifs.R index 993a689c200..37ef2b3a1b8 100644 --- a/R/motifs.R +++ b/R/motifs.R @@ -216,7 +216,7 @@ count_motifs <- function(graph, size = 3, cut.prob = rep(0, size)) { #' 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.size = ceiling(vcount(graph) / 10), sample = NULL) { ensure_igraph(graph) cut.prob <- as.numeric(cut.prob) if (length(cut.prob) != size) { @@ -226,10 +226,14 @@ sample_motifs <- function(graph, size = 3, cut.prob = rep(0, size), ) } + if (!is.null(sample)) { + sample <- as_igraph_vs(graph, sample) - 1 + } + 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 ) } diff --git a/man/sample_motifs.Rd b/man/sample_motifs.Rd index 3d9eac4f8e1..eb01e15173e 100644 --- a/man/sample_motifs.Rd +++ b/man/sample_motifs.Rd @@ -8,7 +8,7 @@ sample_motifs( graph, size = 3, cut.prob = rep(0, size), - sample.size = vcount(graph)/10, + sample.size = ceiling(vcount(graph)/10), sample = NULL ) } From 1950bab584c47c91fe20402f334d425d3c01e9eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20Horva=CC=81t?= Date: Sun, 3 Nov 2024 12:15:54 +0000 Subject: [PATCH 2/5] test: add smoke test for `sample_motifs()` --- tests/testthat/test-motifs.R | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/testthat/test-motifs.R b/tests/testthat/test-motifs.R index a72c230eb8d..4e98de7c3f1 100644 --- a/tests/testthat/test-motifs.R +++ b/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) + + mc <- sample_motifs(g) + expect_true(0 <= mc && mc <= n*(n-1)*(n-2) / 6) + + mc <- sample_motifs(g, sample = c("C", "D", "E", "F")) + expect_true(0 <= mc && mc <= n*(n-1)*(n-2) / 6) + + mc <- sample_motifs(g, sample = V(g)) + expect_true(0 <= mc && mc <= n*(n-1)*(n-2) / 6) +}) From dca5e2fb1ba1acf6f19673e4aba74c884e465085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Thu, 7 Nov 2024 10:31:59 +0100 Subject: [PATCH 3/5] NULL default --- R/motifs.R | 17 ++++++++++++++--- man/graph.motifs.est.Rd | 3 ++- man/sample_motifs.Rd | 5 +++-- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/R/motifs.R b/R/motifs.R index 37ef2b3a1b8..5414a06712c 100644 --- a/R/motifs.R +++ b/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 = ceiling(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,8 +232,13 @@ sample_motifs <- function(graph, size = 3, cut.prob = rep(0, size), ) } - if (!is.null(sample)) { + 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)) diff --git a/man/graph.motifs.est.Rd b/man/graph.motifs.est.Rd index 82dce6287d4..0fdc61c3971 100644 --- a/man/graph.motifs.est.Rd +++ b/man/graph.motifs.est.Rd @@ -23,7 +23,8 @@ graph is cut at a certain level. Its length should be the same as the size of the motif (the \code{size} argument). By default no cuts are made.} \item{sample.size}{The number of vertices to use as a starting point for -finding motifs. Only used if the \code{sample} argument is \code{NULL}.} +finding motifs. Only used if the \code{sample} argument is \code{NULL}. +The default is \code{ceiling(vcount(graph) / 10)} .} \item{sample}{If not \code{NULL} then it specifies the vertices to use as a starting point for finding motifs.} diff --git a/man/sample_motifs.Rd b/man/sample_motifs.Rd index eb01e15173e..a7b50afbd0f 100644 --- a/man/sample_motifs.Rd +++ b/man/sample_motifs.Rd @@ -8,7 +8,7 @@ sample_motifs( graph, size = 3, cut.prob = rep(0, size), - sample.size = ceiling(vcount(graph)/10), + sample.size = NULL, sample = NULL ) } @@ -23,7 +23,8 @@ graph is cut at a certain level. Its length should be the same as the size of the motif (the \code{size} argument). By default no cuts are made.} \item{sample.size}{The number of vertices to use as a starting point for -finding motifs. Only used if the \code{sample} argument is \code{NULL}.} +finding motifs. Only used if the \code{sample} argument is \code{NULL}. +The default is \code{ceiling(vcount(graph) / 10)} .} \item{sample}{If not \code{NULL} then it specifies the vertices to use as a starting point for finding motifs.} From af027c92bd64ac0dffad523ac715cd4d1451cb1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Thu, 7 Nov 2024 10:49:59 +0100 Subject: [PATCH 4/5] Rename --- tests/testthat/test-motifs.R | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-motifs.R b/tests/testthat/test-motifs.R index 4e98de7c3f1..3d85c2c95b0 100644 --- a/tests/testthat/test-motifs.R +++ b/tests/testthat/test-motifs.R @@ -51,12 +51,12 @@ test_that("sample_motifs works", { g <- make_graph(~ A-B-C-A-D-E-F-D-C-F) n <- vcount(g) - mc <- sample_motifs(g) - expect_true(0 <= mc && mc <= n*(n-1)*(n-2) / 6) + motif_count <- sample_motifs(g) + expect_true(0 <= motif_count && motif_count <= n*(n-1)*(n-2) / 6) - mc <- sample_motifs(g, sample = c("C", "D", "E", "F")) - expect_true(0 <= mc && mc <= 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) - mc <- sample_motifs(g, sample = V(g)) - expect_true(0 <= mc && mc <= 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) }) From 751f59385a2243ab656e6d2a6e75693e25c3c542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Thu, 7 Nov 2024 14:21:58 +0100 Subject: [PATCH 5/5] Skip rgl checks in sanitizer --- tests/testthat/test-plot.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-plot.R b/tests/testthat/test-plot.R index b1100a7cbea..66f295269a0 100644 --- a/tests/testthat/test-plot.R +++ b/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)