diff --git a/NAMESPACE b/NAMESPACE index 19c88795d6..f0cc95bf2c 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -937,6 +937,7 @@ importFrom(rlang,"%||%") importFrom(rlang,.data) importFrom(rlang,.env) importFrom(rlang,as_function) +importFrom(rlang,caller_env) importFrom(rlang,check_dots_empty) importFrom(rlang,check_installed) importFrom(rlang,global_env) diff --git a/R/adjacency.R b/R/adjacency.R index 9ebf52700c..84ded07597 100644 --- a/R/adjacency.R +++ b/R/adjacency.R @@ -228,6 +228,7 @@ graph_from_adjacency_matrix <- function(adjmatrix, ), weighted = NULL, diag = TRUE, add.colnames = NULL, add.rownames = NA) { + ensure_no_na(adjmatrix, "adjacency matrix") mode <- igraph.match.arg(mode) if (!is.matrix(adjmatrix) && !inherits(adjmatrix, "Matrix")) { diff --git a/R/conversion.R b/R/conversion.R index b68a3975df..839afc1616 100644 --- a/R/conversion.R +++ b/R/conversion.R @@ -1397,10 +1397,8 @@ graph_from_data_frame <- function(d, directed = TRUE, vertices = NULL) { } ## Handle if some elements are 'NA' - if (any(is.na(d[, 1:2]))) { - cli::cli_warn("In {.code d}, {.code NA} elements were replaced with string {.str NA}.") - d[, 1:2][is.na(d[, 1:2])] <- "NA" - } + ensure_no_na(d, "edge data frame") + if (!is.null(vertices) && any(is.na(vertices[, 1]))) { cli::cli_warn("In {.code vertices[,1]}, {.code NA} elements were replaced with string {.str NA}.") vertices[, 1][is.na(vertices[, 1])] <- "NA" @@ -1489,8 +1487,9 @@ from_data_frame <- function(...) constructor_spec(graph_from_data_frame, ...) #' graph_from_edgelist(cbind(1:10, c(2:10, 1))) graph_from_edgelist <- function(el, directed = TRUE) { if (!is.matrix(el) || ncol(el) != 2) { - cli::cli_abort("graph_from_edgelist expects a matrix with two columns") + cli::cli_abort("graph_from_edgelist expects a matrix with two columns.") } + ensure_no_na(el, "edgelist") if (nrow(el) == 0) { res <- make_empty_graph(directed = directed) diff --git a/R/incidence.R b/R/incidence.R index dd1118b735..9a3c90010a 100644 --- a/R/incidence.R +++ b/R/incidence.R @@ -168,6 +168,7 @@ graph_from_biadjacency_matrix <- function(incidence, directed = FALSE, multiple = FALSE, weighted = NULL, add.names = NULL) { # Argument checks + ensure_no_na(incidence, "biadjacency matrix") directed <- as.logical(directed) mode <- igraph.match.arg(mode) diff --git a/R/utils-assert-args.R b/R/utils-assert-args.R index 3bb7d16736..e66cd3b31d 100644 --- a/R/utils-assert-args.R +++ b/R/utils-assert-args.R @@ -32,3 +32,10 @@ igraph.match.arg <- function(arg, values, error_call = rlang::caller_env()) { error_call = error_call ) } + +#' @importFrom rlang caller_env +ensure_no_na <- function(x, what, call = caller_env()) { + if (any(is.na(x))) { + cli::cli_abort("Cannot create a graph object because the {what} contains NAs.", call = call) + } +} diff --git a/tests/testthat/_snaps/adjacency.md b/tests/testthat/_snaps/adjacency.md index c46c414628..1af5c9cbce 100644 --- a/tests/testthat/_snaps/adjacency.md +++ b/tests/testthat/_snaps/adjacency.md @@ -139,3 +139,11 @@ IGRAPH U--- 2 0 -- + edges: +# graph_from_adjacency_matrix errors for NAs + + Code + graph_from_adjacency_matrix(A) + Condition + Error in `graph_from_adjacency_matrix()`: + ! Cannot create a graph object because the adjacency matrix contains NAs. + diff --git a/tests/testthat/_snaps/conversion.md b/tests/testthat/_snaps/conversion.md index 8da10b7890..33624b7d48 100644 --- a/tests/testthat/_snaps/conversion.md +++ b/tests/testthat/_snaps/conversion.md @@ -87,3 +87,11 @@ 2 2 3 2 b B c C 3 1 3 1 a A c C +# graph_from_edgelist errors for NAs + + Code + graph_from_edgelist(A) + Condition + Error: + ! Cannot create a graph object because the edgelist contains NAs. + diff --git a/tests/testthat/_snaps/incidence.md b/tests/testthat/_snaps/incidence.md index 2b9e6ff16a..dbb1ba9a91 100644 --- a/tests/testthat/_snaps/incidence.md +++ b/tests/testthat/_snaps/incidence.md @@ -85,3 +85,11 @@ ! `multiple` and `weighted` cannot be both `TRUE`. igraph either interprets numbers larger than 1 as weights or as multiplicities, but it cannot be both. +# graph_from_biadjacency_matrix errors for NAs + + Code + graph_from_biadjacency_matrix(A) + Condition + Error in `graph_from_biadjacency_matrix()`: + ! Cannot create a graph object because the biadjacency matrix contains NAs. + diff --git a/tests/testthat/test-adjacency.R b/tests/testthat/test-adjacency.R index 2e9983b99f..0812b24973 100644 --- a/tests/testthat/test-adjacency.R +++ b/tests/testthat/test-adjacency.R @@ -670,7 +670,6 @@ test_that("sparse/dense matrices no loops works", { g <- graph_from_adjacency_matrix(A, diag = FALSE) expect_ecount(g, 1) expect_equal(get_edge_ids(g, c(1, 2)), 1) - }) test_that("sparse/dense matrices multiple works", { @@ -681,11 +680,10 @@ test_that("sparse/dense matrices multiple works", { expect_ecount(g, 3) expect_equal(as_edgelist(g), matrix(c(1, 2), 3, 2, byrow = TRUE)) - A <- as(A,"dgCMatrix") + A <- as(A, "dgCMatrix") g <- graph_from_adjacency_matrix(A, diag = FALSE) expect_ecount(g, 3) expect_equal(as_edgelist(g), matrix(c(1, 2), 3, 2, byrow = TRUE)) - }) test_that("sparse/dense matrices min/max/plus", { @@ -712,3 +710,8 @@ test_that("sparse/dense matrices min/max/plus", { g <- graph_from_adjacency_matrix(A, diag = FALSE, mode = "plus", weighted = TRUE) expect_equal(E(g)$weight[1], 5) }) + +test_that("graph_from_adjacency_matrix errors for NAs", { + A <- matrix(c(1, 1, NA, 1), 2, 2) + expect_snapshot(graph_from_adjacency_matrix(A), error = TRUE) +}) diff --git a/tests/testthat/test-conversion.R b/tests/testthat/test-conversion.R index bf8f5e97b6..b097db125f 100644 --- a/tests/testthat/test-conversion.R +++ b/tests/testthat/test-conversion.R @@ -597,6 +597,9 @@ test_that("edge names work", { ) }) +test_that("graph_from_edgelist errors for NAs", { + A <- matrix(c(1, 2, NA, 1), 2, 2) + expect_snapshot(graph_from_edgelist(A), error = TRUE) test_that("graph_from_data_frame works with factors", { actors <- data.frame( name = c("Alice", "Bob", "Cecil", "David", "Esmeralda"), diff --git a/tests/testthat/test-incidence.R b/tests/testthat/test-incidence.R index 47034a9deb..9966a8986c 100644 --- a/tests/testthat/test-incidence.R +++ b/tests/testthat/test-incidence.R @@ -175,3 +175,8 @@ test_that("graph_from_biadjacency_matrix() errors well", { (g <- graph_from_biadjacency_matrix(inc, multiple = TRUE, weighted = TRUE)) }) }) + +test_that("graph_from_biadjacency_matrix errors for NAs", { + A <- matrix(c(1, 1, NA, 1), 2, 2) + expect_snapshot(graph_from_biadjacency_matrix(A), error = TRUE) +})