From 48ba1befaaa39b4901b88f19f2cdb621cae6c81f Mon Sep 17 00:00:00 2001 From: schochastics Date: Mon, 3 Mar 2025 08:46:40 +0100 Subject: [PATCH 1/4] remove recycling of logical indices --- R/iterators.R | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/R/iterators.R b/R/iterators.R index 2a3ce7ae6d3..aefd63c4fd2 100644 --- a/R/iterators.R +++ b/R/iterators.R @@ -676,6 +676,10 @@ simple_vs_index <- function(x, i, na_ok = FALSE) { if (is.null(ii)) { return(NULL) } + if (is.logical(ii) && length(ii) != length(x)) { + cli::cli_abort("Error: Logical index length does not match the number of vertices. Recycling is not allowed.") + } + ii <- simple_vs_index(x, ii, na_ok) attr(ii, "env") <- attr(x, "env") attr(ii, "graph") <- attr(x, "graph") @@ -991,6 +995,10 @@ simple_es_index <- function(x, i, na_ok = FALSE) { if (is.null(ii)) { return(NULL) } + if (is.logical(ii) && length(ii) != length(x)) { + cli::cli_abort("Error: Logical index length does not match the number of edges. Recycling is not allowed.") + } + ii <- simple_es_index(x, ii) attr(ii, "env") <- attr(x, "env") attr(ii, "graph") <- attr(x, "graph") From 288eb726c37015e854dde441232b0f2df27d2e95 Mon Sep 17 00:00:00 2001 From: schochastics Date: Mon, 3 Mar 2025 09:13:09 +0100 Subject: [PATCH 2/4] removed support for logical index recycling --- R/iterators.R | 7 +++++-- tests/testthat/_snaps/iterators.md | 20 ++++++++++++++++++++ tests/testthat/test-iterators.R | 7 +++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/R/iterators.R b/R/iterators.R index aefd63c4fd2..24df897f8a1 100644 --- a/R/iterators.R +++ b/R/iterators.R @@ -676,7 +676,7 @@ simple_vs_index <- function(x, i, na_ok = FALSE) { if (is.null(ii)) { return(NULL) } - if (is.logical(ii) && length(ii) != length(x)) { + if (is.logical(ii) && (length(ii) != length(x) && length(ii) != 1)) { cli::cli_abort("Error: Logical index length does not match the number of vertices. Recycling is not allowed.") } @@ -995,7 +995,10 @@ simple_es_index <- function(x, i, na_ok = FALSE) { if (is.null(ii)) { return(NULL) } - if (is.logical(ii) && length(ii) != length(x)) { + if (is.logical(ii) && (length(ii) != length(x) && length(ii) != 1)) { + print(ii) + print(length(ii)) + print(length(x)) cli::cli_abort("Error: Logical index length does not match the number of edges. Recycling is not allowed.") } diff --git a/tests/testthat/_snaps/iterators.md b/tests/testthat/_snaps/iterators.md index 4deef4e697b..ed213c2e8f4 100644 --- a/tests/testthat/_snaps/iterators.md +++ b/tests/testthat/_snaps/iterators.md @@ -92,3 +92,23 @@ + 10/? edges (deleted) (vertex names): [1] a|b b|c c|d d|e e|f f|g g|h h|i i|j a|j +# logical indices are not recycled + + Code + V(g)[c(TRUE, FALSE)] + Condition + Error in `FUN()`: + ! Error: Logical index length does not match the number of vertices. Recycling is not allowed. + +--- + + Code + E(g)[c(TRUE, FALSE)] + Output + [1] TRUE FALSE + [1] 2 + [1] 5 + Condition + Error in `FUN()`: + ! Error: Logical index length does not match the number of edges. Recycling is not allowed. + diff --git a/tests/testthat/test-iterators.R b/tests/testthat/test-iterators.R index 9df5fdadf8b..abb487d41cc 100644 --- a/tests/testthat/test-iterators.R +++ b/tests/testthat/test-iterators.R @@ -440,3 +440,10 @@ test_that("edge indexes are stored as raw numbers", { expect_identical(E(g)$id, as.numeric(1:3)) expect_error(induced_subgraph(g, 1:2), NA) }) + +test_that("logical indices are not recycled", { + # https://github.com/igraph/rigraph/issues/848 + g <- make_ring(5) + expect_snapshot(V(g)[c(TRUE, FALSE)], error = TRUE) + expect_snapshot(E(g)[c(TRUE, FALSE)], error = TRUE) +}) From e08a08f4c58f6f4bc201b9d6d4dcd983cf8d9777 Mon Sep 17 00:00:00 2001 From: schochastics Date: Mon, 3 Mar 2025 09:16:00 +0100 Subject: [PATCH 3/4] remove print debugging --- R/iterators.R | 3 --- 1 file changed, 3 deletions(-) diff --git a/R/iterators.R b/R/iterators.R index 24df897f8a1..b74f0dcb6d1 100644 --- a/R/iterators.R +++ b/R/iterators.R @@ -996,9 +996,6 @@ simple_es_index <- function(x, i, na_ok = FALSE) { return(NULL) } if (is.logical(ii) && (length(ii) != length(x) && length(ii) != 1)) { - print(ii) - print(length(ii)) - print(length(x)) cli::cli_abort("Error: Logical index length does not match the number of edges. Recycling is not allowed.") } From 25573dd08669ef739c96445a10e4025e98ef9eb0 Mon Sep 17 00:00:00 2001 From: schochastics Date: Mon, 3 Mar 2025 10:50:32 +0100 Subject: [PATCH 4/4] fixed failing snapshot --- tests/testthat/_snaps/iterators.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/testthat/_snaps/iterators.md b/tests/testthat/_snaps/iterators.md index ed213c2e8f4..d01392169aa 100644 --- a/tests/testthat/_snaps/iterators.md +++ b/tests/testthat/_snaps/iterators.md @@ -104,10 +104,6 @@ Code E(g)[c(TRUE, FALSE)] - Output - [1] TRUE FALSE - [1] 2 - [1] 5 Condition Error in `FUN()`: ! Error: Logical index length does not match the number of edges. Recycling is not allowed.