From 80ca590e53f41985dcc0fb4ad2a710c2f80b509e Mon Sep 17 00:00:00 2001 From: schochastics Date: Fri, 20 Sep 2024 21:06:59 +0200 Subject: [PATCH 01/11] removed for loops in get.adjacency.dense --- R/conversion.R | 63 ++++++++------------------------------------------ 1 file changed, 9 insertions(+), 54 deletions(-) diff --git a/R/conversion.R b/R/conversion.R index ec548d0129..f31e47c5b4 100644 --- a/R/conversion.R +++ b/R/conversion.R @@ -1,4 +1,3 @@ - #' Convert igraph graphs to graphNEL objects from the graph package #' #' @description @@ -159,11 +158,6 @@ get.adjacency.dense <- function(graph, type = c("both", "upper", "lower"), ensure_igraph(graph) type <- igraph.match.arg(type) - type <- switch(type, - "upper" = 0, - "lower" = 1, - "both" = 2 - ) if (is.logical(loops)) { loops <- ifelse(loops, "once", "ignore") @@ -183,61 +177,22 @@ get.adjacency.dense <- function(graph, type = c("both", "upper", "lower"), if (is.null(attr)) { on.exit(.Call(R_igraph_finalizer)) + type <- switch(type, + "upper" = 0, + "lower" = 1, + "both" = 2 + ) res <- .Call( R_igraph_get_adjacency, graph, as.numeric(type), weights, loops ) } else { - attr <- as.character(attr) - if (!attr %in% edge_attr_names(graph)) { - stop("no such edge attribute") - } - exattr <- edge_attr(graph, attr) - if (is.logical(exattr)) { - res <- matrix(FALSE, nrow = vcount(graph), ncol = vcount(graph)) - } else if (is.numeric(exattr)) { - res <- matrix(0, nrow = vcount(graph), ncol = vcount(graph)) - } else { - stop( - "Matrices must be either numeric or logical, ", - "and the edge attribute is not" - ) - } - if (is_directed(graph)) { - for (i in seq(length.out = ecount(graph))) { - e <- ends(graph, i, names = FALSE) - res[e[1], e[2]] <- exattr[i] - } - } else { - if (type == 0) { - ## upper - for (i in seq(length.out = ecount(graph))) { - e <- ends(graph, i, names = FALSE) - res[min(e), max(e)] <- exattr[i] - } - } else if (type == 1) { - ## lower - for (i in seq(length.out = ecount(graph))) { - e <- ends(graph, i, names = FALSE) - res[max(e), min(e)] <- exattr[i] - } - } else if (type == 2) { - ## both - for (i in seq(length.out = ecount(graph))) { - e <- ends(graph, i, names = FALSE) - res[e[1], e[2]] <- exattr[i] - if (e[1] != e[2]) { - res[e[2], e[1]] <- exattr[i] - } - } - } - } + res <- as.matrix(get.adjacency.sparse(graph, type = type, attr = attr, names = names)) } if (names && "name" %in% vertex_attr_names(graph)) { colnames(res) <- rownames(res) <- V(graph)$name } - res } @@ -1004,7 +959,7 @@ get.incidence.sparse <- function(graph, types, names, attr) { #' as_biadjacency_matrix(g) #' as_biadjacency_matrix <- function(graph, types = NULL, attr = NULL, - names = TRUE, sparse = FALSE) { + names = TRUE, sparse = FALSE) { # Argument checks ensure_igraph(graph) types <- handle_vertex_type_arg(types, graph) @@ -1033,8 +988,8 @@ as_biadjacency_matrix <- function(graph, types = NULL, attr = NULL, #' this naming to avoid confusion with the edge-vertex incidence matrix. #' @export as_incidence_matrix <- function(...) { # nocov start - lifecycle::deprecate_soft("1.6.0", "as_incidence_matrix()", "as_biadjacency_matrix()") - as_biadjacency_matrix(...) + lifecycle::deprecate_soft("1.6.0", "as_incidence_matrix()", "as_biadjacency_matrix()") + as_biadjacency_matrix(...) } # nocov end #' @rdname graph_from_data_frame #' @param x An igraph object. From 78403ce6c06621c2b289472b703a788e8066fb0b Mon Sep 17 00:00:00 2001 From: schochastics Date: Wed, 9 Oct 2024 21:01:38 +0200 Subject: [PATCH 02/11] added comment that as.matrix on sparse adj is fastest solution --- R/conversion.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/conversion.R b/R/conversion.R index f31e47c5b4..e155697595 100644 --- a/R/conversion.R +++ b/R/conversion.R @@ -187,6 +187,7 @@ get.adjacency.dense <- function(graph, type = c("both", "upper", "lower"), loops ) } else { + # faster than a specialized implementation res <- as.matrix(get.adjacency.sparse(graph, type = type, attr = attr, names = names)) } From 16a7fe138e161764c954adb234283db5a7e1c587 Mon Sep 17 00:00:00 2001 From: schochastics Date: Mon, 13 Jan 2025 21:11:11 +0100 Subject: [PATCH 03/11] fix failing tests due to #1551 --- tests/testthat/_snaps/conversion.md | 4 +-- tests/testthat/test-conversion.R | 50 +++++++++++++++++++---------- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/tests/testthat/_snaps/conversion.md b/tests/testthat/_snaps/conversion.md index 12b0bc0c7f..98423b4262 100644 --- a/tests/testthat/_snaps/conversion.md +++ b/tests/testthat/_snaps/conversion.md @@ -41,7 +41,7 @@ Code as_adjacency_matrix(g, attr = "bla", sparse = FALSE) Condition - Error in `get.adjacency.dense()`: + Error in `get.adjacency.sparse()`: ! no such edge attribute --- @@ -49,6 +49,6 @@ Code as_adjacency_matrix(g, attr = "bla", sparse = FALSE) Condition - Error in `get.adjacency.dense()`: + Error in `get.adjacency.sparse()`: ! Matrices must be either numeric or logical, and the edge attribute is not diff --git a/tests/testthat/test-conversion.R b/tests/testthat/test-conversion.R index aa8ca51fe7..e8278b06c5 100644 --- a/tests/testthat/test-conversion.R +++ b/tests/testthat/test-conversion.R @@ -71,7 +71,7 @@ test_that("as_undirected() keeps attributes", { }) test_that("as_adjacency_matrix() works -- sparse", { - g <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed = TRUE) + g <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE) basic_adj_matrix <- as_adjacency_matrix(g) expect_s4_class(basic_adj_matrix, "dgCMatrix") expected_matrix <- matrix( @@ -93,17 +93,19 @@ test_that("as_adjacency_matrix() works -- sparse", { E(g)$weight <- c(1.2, 3.4, 2.7, 5.6, 6.0, 0.1, 6.1, 3.3, 4.3) weight_adj_matrix <- as_adjacency_matrix(g, attr = "weight") expect_s4_class(weight_adj_matrix, "dgCMatrix") - expect_equal(as.matrix(weight_adj_matrix), + expect_equal( + as.matrix(weight_adj_matrix), matrix( c(0, 3.4, 0, 0, 1.2, 2.7, 0, 13.7, 0, 0, 11.6, 0, 0, 0, 0.1, 0), nrow = 4L, ncol = 4L, dimnames = list(c("a", "b", "c", "d"), c("a", "b", "c", "d")) - )) + ) + ) }) test_that("as_adjacency_matrix() works -- sparse + not both", { - dg <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed = TRUE) + dg <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE) g <- as_undirected(dg, mode = "each") lower_adj_matrix <- as_adjacency_matrix(g, type = "lower") @@ -128,16 +130,15 @@ test_that("as_adjacency_matrix() works -- sparse + not both", { }) test_that("as_adjacency_matrix() errors well -- sparse", { - g <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed = TRUE) + g <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE) expect_snapshot(as_adjacency_matrix(g, attr = "bla"), error = TRUE) E(g)$bla <- letters[1:ecount(g)] expect_snapshot(as_adjacency_matrix(g, attr = "bla"), error = TRUE) - }) test_that("as_adjacency_matrix() works -- sparse undirected", { - dg <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed = TRUE) + dg <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE) ug <- as_undirected(dg, mode = "each") adj_matrix <- as_adjacency_matrix(ug) expect_s4_class(adj_matrix, "dgCMatrix") @@ -155,7 +156,7 @@ test_that("as_adjacency_matrix() works -- sparse undirected", { }) test_that("as_adjacency_matrix() works -- dense", { - g <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed = TRUE) + g <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE) basic_adj_matrix <- as_adjacency_matrix(g, sparse = FALSE) expected_matrix <- matrix( @@ -175,7 +176,11 @@ test_that("as_adjacency_matrix() works -- dense", { expect_equal( weight_adj_matrix, matrix( - c(0, 3.4, 0, 0, 1.2, 2.7, 0, 4.3, 0, 0, 6, 0, 0, 0, 0.1, 0), + c(0, 3.4, 0, 0, 1.2, 2.7, 0, 13.7, 0, 0, 11.6, 0, 0, 0, 0.1, 0), + # below is wrong test result due to a bug (#1551). Weights of ties + # between the same node pair should be aggregated and not only the last + # weight should be considered. The above is consistent with the sparse case + # c(0, 3.4, 0, 0, 1.2, 2.7, 0, 4.3, 0, 0, 6, 0, 0, 0, 0.1, 0), nrow = 4L, ncol = 4L, dimnames = list(c("a", "b", "c", "d"), c("a", "b", "c", "d")) @@ -184,17 +189,16 @@ test_that("as_adjacency_matrix() works -- dense", { }) test_that("as_adjacency_matrix() errors well -- dense", { - g <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed = TRUE) + g <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE) expect_snapshot(as_adjacency_matrix(g, attr = "bla", sparse = FALSE), error = TRUE) E(g)$bla <- letters[1:ecount(g)] expect_snapshot(as_adjacency_matrix(g, attr = "bla", sparse = FALSE), error = TRUE) - }) test_that("as_adjacency_matrix() works -- dense undirected", { - dg <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed = TRUE) + dg <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE) ug <- as_undirected(dg, mode = "each") # no different treatment than undirected if no attribute?! adj_matrix <- as_adjacency_matrix(ug, sparse = FALSE) @@ -211,7 +215,11 @@ test_that("as_adjacency_matrix() works -- dense undirected", { expect_equal( weight_adj_matrix, matrix( - c(0, 3.4, 0, 0, 3.4, 2.7, 0, 4.3, 0, 0, 6, 0.1, 0, 4.3, 0.1, 0), + c(0, 4.6, 0, 0, 4.6, 2.7, 0, 13.7, 0, 0, 11.6, 0.1, 0, 13.7, 0.1, 0), + # below is wrong test result due to a bug (#1551). Weights of ties + # between the same node pair should be aggregated and not only the last + # weight should be considered. The above is consistent with the sparse case + # c(0, 3.4, 0, 0, 3.4, 2.7, 0, 4.3, 0, 0, 6, 0.1, 0, 4.3, 0.1, 0), nrow = 4L, ncol = 4L ) @@ -219,7 +227,7 @@ test_that("as_adjacency_matrix() works -- dense undirected", { }) test_that("as_adjacency_matrix() works -- dense + not both", { - dg <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed = TRUE) + dg <- make_graph(c(1, 2, 2, 1, 2, 2, 3, 3, 3, 3, 3, 4, 4, 2, 4, 2, 4, 2), directed = TRUE) g <- as_undirected(dg, mode = "each") E(g)$attribute <- c(1.2, 3.4, 2.7, 5.6, 6.0, 0.1, 6.1, 3.3, 4.3) @@ -233,13 +241,17 @@ test_that("as_adjacency_matrix() works -- dense + not both", { expect_equal( lower_adj_matrix, matrix( - c(0, 3.4, 0, 0, 0, 2.7, 0, 4.3, 0, 0, 6, 0.1, 0, 0, 0, 0), + c(0, 4.6, 0, 0, 0, 2.7, 0, 13.7, 0, 0, 11.6, 0.1, 0, 0, 0, 0), + # below is wrong test result due to a bug (#1551). Weights of ties + # between the same node pair should be aggregated and not only the last + # weight should be considered. The above is consistent with the sparse case + # c(0, 3.4, 0, 0, 0, 2.7, 0, 4.3, 0, 0, 6, 0.1, 0, 0, 0, 0), nrow = 4L, ncol = 4L ) ) - upper_adj_matrix <- as_adjacency_matrix( + upper_adj_matrix <- as_adjacency_matrix( g, type = "upper", sparse = FALSE, @@ -249,7 +261,11 @@ test_that("as_adjacency_matrix() works -- dense + not both", { expect_equal( upper_adj_matrix, matrix( - c(0, 0, 0, 0, 3.4, 2.7, 0, 0, 0, 0, 6, 0, 0, 4.3, 0.1, 0), + c(0, 0, 0, 0, 4.6, 2.7, 0, 0, 0, 0, 11.6, 0, 0, 13.7, 0.1, 0), + # below is wrong test result due to a bug (#1551). Weights of ties + # between the same node pair should be aggregated and not only the last + # weight should be considered. The above is consistent with the sparse case + # c(0, 0, 0, 0, 3.4, 2.7, 0, 0, 0, 0, 6, 0, 0, 4.3, 0.1, 0), nrow = 4L, ncol = 4L ) From 563ed24e838c1faf57b38b921070233ce0a4cbf3 Mon Sep 17 00:00:00 2001 From: schochastics Date: Thu, 16 Jan 2025 13:18:34 +0100 Subject: [PATCH 04/11] added call. = FALSE to stop calls for clearer error reporting --- R/conversion.R | 14 +++++++------- tests/testthat/_snaps/conversion.md | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/R/conversion.R b/R/conversion.R index e155697595..9a38a3dd11 100644 --- a/R/conversion.R +++ b/R/conversion.R @@ -211,13 +211,14 @@ get.adjacency.sparse <- function(graph, type = c("both", "upper", "lower"), if (!is.null(attr)) { attr <- as.character(attr) if (!attr %in% edge_attr_names(graph)) { - stop("no such edge attribute") + stop("no such edge attribute", call. = FALSE) } value <- edge_attr(graph, name = attr) if (!is.numeric(value) && !is.logical(value)) { stop( "Matrices must be either numeric or logical, ", - "and the edge attribute is not" + "and the edge attribute is not", + call. = FALSE ) } } else { @@ -334,7 +335,6 @@ as_adjacency_matrix <- function(graph, type = c("both", "upper", "lower"), as_adj <- function(graph, type = c("both", "upper", "lower"), attr = NULL, edges = deprecated(), names = TRUE, sparse = igraph_opt("sparsematrices")) { - lifecycle::deprecate_soft("2.1.0", "as_adj()", "as_adjacency_matrix()") as_adjacency_matrix( @@ -834,7 +834,7 @@ get.incidence.dense <- function(graph, types, names, attr) { } else { attr <- as.character(attr) if (!attr %in% edge_attr_names(graph)) { - stop("no such edge attribute") + stop("no such edge attribute", call. = FALSE) } vc <- vcount(graph) @@ -871,12 +871,12 @@ get.incidence.dense <- function(graph, types, names, attr) { get.incidence.sparse <- function(graph, types, names, attr) { vc <- vcount(graph) if (length(types) != vc) { - stop("Invalid types vector") + stop("Invalid types vector", call. = FALSE) } el <- as_edgelist(graph, names = FALSE) if (any(types[el[, 1]] == types[el[, 2]])) { - stop("Invalid types vector, not a bipartite graph") + stop("Invalid types vector, not a bipartite graph", call. = FALSE) } n1 <- sum(!types) @@ -896,7 +896,7 @@ get.incidence.sparse <- function(graph, types, names, attr) { if (!is.null(attr)) { attr <- as.character(attr) if (!attr %in% edge_attr_names(graph)) { - stop("no such edge attribute") + stop("no such edge attribute", call. = FALSE) } value <- edge_attr(graph, name = attr) } else { diff --git a/tests/testthat/_snaps/conversion.md b/tests/testthat/_snaps/conversion.md index 98423b4262..66ee0de5fc 100644 --- a/tests/testthat/_snaps/conversion.md +++ b/tests/testthat/_snaps/conversion.md @@ -25,7 +25,7 @@ Code as_adjacency_matrix(g, attr = "bla") Condition - Error in `get.adjacency.sparse()`: + Error: ! no such edge attribute --- @@ -33,7 +33,7 @@ Code as_adjacency_matrix(g, attr = "bla") Condition - Error in `get.adjacency.sparse()`: + Error: ! Matrices must be either numeric or logical, and the edge attribute is not # as_adjacency_matrix() errors well -- dense @@ -41,7 +41,7 @@ Code as_adjacency_matrix(g, attr = "bla", sparse = FALSE) Condition - Error in `get.adjacency.sparse()`: + Error: ! no such edge attribute --- @@ -49,6 +49,6 @@ Code as_adjacency_matrix(g, attr = "bla", sparse = FALSE) Condition - Error in `get.adjacency.sparse()`: + Error: ! Matrices must be either numeric or logical, and the edge attribute is not From 1ecedd89242950ae1c15115ba8be3cae517e801d Mon Sep 17 00:00:00 2001 From: schochastics Date: Thu, 16 Jan 2025 13:34:44 +0100 Subject: [PATCH 05/11] fixed failing tests on R 4.0 --- tests/testthat/test-conversion.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-conversion.R b/tests/testthat/test-conversion.R index e8278b06c5..3ff70fd7f1 100644 --- a/tests/testthat/test-conversion.R +++ b/tests/testthat/test-conversion.R @@ -202,6 +202,7 @@ test_that("as_adjacency_matrix() works -- dense undirected", { ug <- as_undirected(dg, mode = "each") # no different treatment than undirected if no attribute?! adj_matrix <- as_adjacency_matrix(ug, sparse = FALSE) + dimnames(adj_matrix) <- NULL expect_equal( adj_matrix, matrix( @@ -212,6 +213,7 @@ test_that("as_adjacency_matrix() works -- dense undirected", { E(ug)$weight <- c(1.2, 3.4, 2.7, 5.6, 6.0, 0.1, 6.1, 3.3, 4.3) weight_adj_matrix <- as_adjacency_matrix(ug, sparse = FALSE, attr = "weight") + dimnames(weight_adj_matrix) <- NULL expect_equal( weight_adj_matrix, matrix( @@ -237,6 +239,7 @@ test_that("as_adjacency_matrix() works -- dense + not both", { sparse = FALSE, attr = "attribute" ) + dimnames(lower_adj_matrix) <- NULL expect_equal( lower_adj_matrix, @@ -257,7 +260,7 @@ test_that("as_adjacency_matrix() works -- dense + not both", { sparse = FALSE, attr = "attribute" ) - + dimnames(upper_adj_matrix) <- NULL expect_equal( upper_adj_matrix, matrix( From d303690ac4bc51ede59fd4b7ee1846f4611ccb03 Mon Sep 17 00:00:00 2001 From: schochastics Date: Tue, 21 Jan 2025 12:36:53 +0100 Subject: [PATCH 06/11] switched to cli::abort error handling --- R/conversion.R | 13 ++++++------- tests/testthat/_snaps/conversion.md | 8 ++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/R/conversion.R b/R/conversion.R index 9a38a3dd11..42076c576b 100644 --- a/R/conversion.R +++ b/R/conversion.R @@ -188,7 +188,7 @@ get.adjacency.dense <- function(graph, type = c("both", "upper", "lower"), ) } else { # faster than a specialized implementation - res <- as.matrix(get.adjacency.sparse(graph, type = type, attr = attr, names = names)) + res <- as.matrix(get.adjacency.sparse(graph, type = type, attr = attr, names = names, call = rlang::caller_env())) } if (names && "name" %in% vertex_attr_names(graph)) { @@ -198,7 +198,7 @@ get.adjacency.dense <- function(graph, type = c("both", "upper", "lower"), } get.adjacency.sparse <- function(graph, type = c("both", "upper", "lower"), - attr = NULL, names = TRUE) { + attr = NULL, names = TRUE, call = rlang::caller_env()) { ensure_igraph(graph) type <- igraph.match.arg(type) @@ -211,14 +211,13 @@ get.adjacency.sparse <- function(graph, type = c("both", "upper", "lower"), if (!is.null(attr)) { attr <- as.character(attr) if (!attr %in% edge_attr_names(graph)) { - stop("no such edge attribute", call. = FALSE) + cli::cli_abort("no such edge attribute", call = call) } value <- edge_attr(graph, name = attr) if (!is.numeric(value) && !is.logical(value)) { - stop( - "Matrices must be either numeric or logical, ", - "and the edge attribute is not", - call. = FALSE + cli::cli_abort( + "Matrices must be either numeric or logical, and the edge attribute is not", + call = call ) } } else { diff --git a/tests/testthat/_snaps/conversion.md b/tests/testthat/_snaps/conversion.md index 66ee0de5fc..6c33bfbd57 100644 --- a/tests/testthat/_snaps/conversion.md +++ b/tests/testthat/_snaps/conversion.md @@ -25,7 +25,7 @@ Code as_adjacency_matrix(g, attr = "bla") Condition - Error: + Error in `as_adjacency_matrix()`: ! no such edge attribute --- @@ -33,7 +33,7 @@ Code as_adjacency_matrix(g, attr = "bla") Condition - Error: + Error in `as_adjacency_matrix()`: ! Matrices must be either numeric or logical, and the edge attribute is not # as_adjacency_matrix() errors well -- dense @@ -41,7 +41,7 @@ Code as_adjacency_matrix(g, attr = "bla", sparse = FALSE) Condition - Error: + Error in `as_adjacency_matrix()`: ! no such edge attribute --- @@ -49,6 +49,6 @@ Code as_adjacency_matrix(g, attr = "bla", sparse = FALSE) Condition - Error: + Error in `as_adjacency_matrix()`: ! Matrices must be either numeric or logical, and the edge attribute is not From ce7158d27244c348a4616de49c1392cd59093665 Mon Sep 17 00:00:00 2001 From: schochastics Date: Fri, 24 Jan 2025 19:28:17 +0100 Subject: [PATCH 07/11] changed all stop calls to cli_abort --- R/conversion.R | 16 ++++++++-------- tests/testthat/_snaps/conversion.md | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/R/conversion.R b/R/conversion.R index 42076c576b..7f79bba798 100644 --- a/R/conversion.R +++ b/R/conversion.R @@ -211,7 +211,7 @@ get.adjacency.sparse <- function(graph, type = c("both", "upper", "lower"), if (!is.null(attr)) { attr <- as.character(attr) if (!attr %in% edge_attr_names(graph)) { - cli::cli_abort("no such edge attribute", call = call) + cli::cli_abort("No such edge attribute", call = call) } value <- edge_attr(graph, name = attr) if (!is.numeric(value) && !is.logical(value)) { @@ -816,7 +816,7 @@ as_graphnel <- function(graph) { res } -get.incidence.dense <- function(graph, types, names, attr) { +get.incidence.dense <- function(graph, types, names, attr, call = rlang::caller_env()) { if (is.null(attr)) { on.exit(.Call(R_igraph_finalizer)) ## Function call @@ -833,7 +833,7 @@ get.incidence.dense <- function(graph, types, names, attr) { } else { attr <- as.character(attr) if (!attr %in% edge_attr_names(graph)) { - stop("no such edge attribute", call. = FALSE) + cli::cli_abort("No such edge attribute", call = call) } vc <- vcount(graph) @@ -867,15 +867,15 @@ get.incidence.dense <- function(graph, types, names, attr) { } } -get.incidence.sparse <- function(graph, types, names, attr) { +get.incidence.sparse <- function(graph, types, names, attr, call = rlang::caller_env()) { vc <- vcount(graph) if (length(types) != vc) { - stop("Invalid types vector", call. = FALSE) + cli::cli_abort("Invalid types vector", call = call) } el <- as_edgelist(graph, names = FALSE) if (any(types[el[, 1]] == types[el[, 2]])) { - stop("Invalid types vector, not a bipartite graph", call. = FALSE) + cli::cli_abort("Invalid types vector, not a bipartite graph", call = call) } n1 <- sum(!types) @@ -895,7 +895,7 @@ get.incidence.sparse <- function(graph, types, names, attr) { if (!is.null(attr)) { attr <- as.character(attr) if (!attr %in% edge_attr_names(graph)) { - stop("no such edge attribute", call. = FALSE) + cli::cli_abort("No such edge attribute", call = calls) } value <- edge_attr(graph, name = attr) } else { @@ -968,7 +968,7 @@ as_biadjacency_matrix <- function(graph, types = NULL, attr = NULL, sparse <- as.logical(sparse) if (sparse) { - get.incidence.sparse(graph, types = types, names = names, attr = attr) + get.incidence.sparse(graph, types = types, names = names, attr = attr, call = rlang::caller_env()) } else { get.incidence.dense(graph, types = types, names = names, attr = attr) } diff --git a/tests/testthat/_snaps/conversion.md b/tests/testthat/_snaps/conversion.md index 6c33bfbd57..901eeddd70 100644 --- a/tests/testthat/_snaps/conversion.md +++ b/tests/testthat/_snaps/conversion.md @@ -26,7 +26,7 @@ as_adjacency_matrix(g, attr = "bla") Condition Error in `as_adjacency_matrix()`: - ! no such edge attribute + ! No such edge attribute --- @@ -42,7 +42,7 @@ as_adjacency_matrix(g, attr = "bla", sparse = FALSE) Condition Error in `as_adjacency_matrix()`: - ! no such edge attribute + ! No such edge attribute --- From 0e5da2cf0cc1967bdb5ed0b089d11133af167343 Mon Sep 17 00:00:00 2001 From: schochastics Date: Fri, 24 Jan 2025 19:40:16 +0100 Subject: [PATCH 08/11] added some more tests --- tests/testthat/test-conversion.R | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/testthat/test-conversion.R b/tests/testthat/test-conversion.R index 3ff70fd7f1..1f1ccf0a09 100644 --- a/tests/testthat/test-conversion.R +++ b/tests/testthat/test-conversion.R @@ -274,3 +274,26 @@ test_that("as_adjacency_matrix() works -- dense + not both", { ) ) }) + +test_that("as_adjacency_matrix() works -- dense + weights", { + g <- make_full_graph(5, directed = FALSE) + E(g)$weight <- 1:10 + mat <- matrix(0, 5, 5) + mat[lower.tri(mat)] <- 1:10 + mat <- mat + t(mat) + A <- as_adjacency_matrix(g, attr = "weight", sparse = FALSE) + expect_equal(A, mat) +}) + +test_that("as_biadjacency_matrix() works -- dense + weights", { + g <- make_bipartite_graph(c(0, 1, 0, 1, 0, 0), c(1, 2, 2, 3, 3, 4)) + E(g)$weight <- c(2, 4, 6) + A <- as_biadjacency_matrix(g, attr = "weight", sparse = FALSE) + mat <- matrix( + c(2, 4, 0, 0, 0, 6, 0, 0), + nrow = 4L, + ncol = 2L, + dimnames = list(c("1", "3", "5", "6"), c("2", "4")) + ) + expect_equal(A, mat) +}) From 64884cef1f2ac0eb801fd3fc317528a9463ba3c4 Mon Sep 17 00:00:00 2001 From: schochastics Date: Sat, 25 Jan 2025 07:12:43 +0100 Subject: [PATCH 09/11] fixed a typo --- R/conversion.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/conversion.R b/R/conversion.R index 7c96306d10..808d19ab96 100644 --- a/R/conversion.R +++ b/R/conversion.R @@ -899,7 +899,7 @@ get.incidence.sparse <- function(graph, types, names, attr, call = rlang::caller if (!is.null(attr)) { attr <- as.character(attr) if (!attr %in% edge_attr_names(graph)) { - cli::cli_abort("No such edge attribute", call = calls) + cli::cli_abort("No such edge attribute", call = call) } value <- edge_attr(graph, name = attr) } else { From ada8e5f979ebe697a73a6786396ce480cc851095 Mon Sep 17 00:00:00 2001 From: schochastics Date: Sat, 25 Jan 2025 15:28:19 +0100 Subject: [PATCH 10/11] canonicalize matrices --- tests/testthat/test-conversion.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-conversion.R b/tests/testthat/test-conversion.R index 1f1ccf0a09..d9212902b0 100644 --- a/tests/testthat/test-conversion.R +++ b/tests/testthat/test-conversion.R @@ -282,7 +282,7 @@ test_that("as_adjacency_matrix() works -- dense + weights", { mat[lower.tri(mat)] <- 1:10 mat <- mat + t(mat) A <- as_adjacency_matrix(g, attr = "weight", sparse = FALSE) - expect_equal(A, mat) + expect_equal(canonicalize_matrix(A), mat) }) test_that("as_biadjacency_matrix() works -- dense + weights", { @@ -295,5 +295,5 @@ test_that("as_biadjacency_matrix() works -- dense + weights", { ncol = 2L, dimnames = list(c("1", "3", "5", "6"), c("2", "4")) ) - expect_equal(A, mat) + expect_equal(canonicalize_matrix(A), mat) }) From 630508d451a61e9826aca60e69dc25282f0c1aff Mon Sep 17 00:00:00 2001 From: schochastics Date: Sat, 25 Jan 2025 19:38:44 +0100 Subject: [PATCH 11/11] canonicalize expected result --- tests/testthat/test-conversion.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-conversion.R b/tests/testthat/test-conversion.R index d9212902b0..176b8ff9f0 100644 --- a/tests/testthat/test-conversion.R +++ b/tests/testthat/test-conversion.R @@ -295,5 +295,5 @@ test_that("as_biadjacency_matrix() works -- dense + weights", { ncol = 2L, dimnames = list(c("1", "3", "5", "6"), c("2", "4")) ) - expect_equal(canonicalize_matrix(A), mat) + expect_equal(canonicalize_matrix(A), canonicalize_matrix(mat)) })