From fdfc75c27a3e32d375794ef6030e22ee8fb51bea Mon Sep 17 00:00:00 2001 From: schochastics Date: Thu, 3 Jul 2025 21:11:47 +0200 Subject: [PATCH 1/8] add error for duplicated vertex names --- R/attributes.R | 23 +++++++++++++++++++++++ R/interface.R | 2 +- tests/testthat/_snaps/attributes.md | 8 ++++++++ tests/testthat/test-attributes.R | 8 ++++++++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/R/attributes.R b/R/attributes.R index 7bac81c39fa..6a7be840699 100644 --- a/R/attributes.R +++ b/R/attributes.R @@ -566,6 +566,29 @@ i_set_vertex_attr <- function( igraph_attr_idx_vertex ) + #Uniqueness check for vertex "name" attribute + if (name == "name") { + current_names <- vattrs[["name"]] + if (is.null(current_names)) { + current_names <- rep(NA_character_, vcount(graph)) + } + idx_numeric <- as.numeric(index) + + unaffected <- setdiff(seq_along(current_names), idx_numeric) + names_unaffected <- current_names[unaffected] + + if (length(value) == 1 && length(idx_numeric) > 1) { + value <- rep(value, length(idx_numeric)) + } + + if (any(value %in% names_unaffected)) { + dupes <- value[value %in% names_unaffected] + cli::cli_abort( + "Vertex name{?s} already exist{?s}: {paste(unique(dupes), collapse = ', ')}" + ) + } + } + complete <- is_complete_iterator(index) name_available <- (name %in% names(vattrs)) if (!complete && !name_available) { diff --git a/R/interface.R b/R/interface.R index e5317fd54ed..786cde8c555 100644 --- a/R/interface.R +++ b/R/interface.R @@ -213,7 +213,7 @@ add_vertices <- function(graph, nv, ..., attr = list()) { attrs <- append(attrs, attr) nam <- names(attrs) if (length(attrs) != 0 && (is.null(nam) || any(nam == ""))) { - stop("please supply names for attributes") + cli::cli_abort("please supply names for attributes") } vertices.orig <- vcount(graph) diff --git a/tests/testthat/_snaps/attributes.md b/tests/testthat/_snaps/attributes.md index c9b916fc2c5..6b19b34baf8 100644 --- a/tests/testthat/_snaps/attributes.md +++ b/tests/testthat/_snaps/attributes.md @@ -62,3 +62,11 @@ Error in `set_graph_attr()`: ! `name` must be a single string, not the number 1. +# duplicated vertex names are handled correctly + + Code + add_vertices(g, nv = 2, attr = list(name = c("A", "B"))) + Condition + Error in `i_set_vertex_attr()`: + ! Vertex name already exist: A, B + diff --git a/tests/testthat/test-attributes.R b/tests/testthat/test-attributes.R index 9736d30b062..f621797d675 100644 --- a/tests/testthat/test-attributes.R +++ b/tests/testthat/test-attributes.R @@ -465,3 +465,11 @@ test_that("good error message when not using character", { set_graph_attr(ring, 1, 1) }) }) + +test_that("duplicated vertex names are handled correctly", { + g <- make_empty_graph(4) + V(g)$name <- LETTERS[1:4] + expect_snapshot(error = TRUE, { + add_vertices(g, nv = 2, attr = list(name = c("A", "B"))) + }) +}) From cc23071b00a212879689fb4c83d2a811c8b83b7e Mon Sep 17 00:00:00 2001 From: David Schoch Date: Tue, 15 Jul 2025 15:59:06 +0200 Subject: [PATCH 2/8] Update R/attributes.R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Maëlle Salmon --- R/attributes.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/attributes.R b/R/attributes.R index 6a7be840699..62ccabf1b3a 100644 --- a/R/attributes.R +++ b/R/attributes.R @@ -570,7 +570,7 @@ i_set_vertex_attr <- function( if (name == "name") { current_names <- vattrs[["name"]] if (is.null(current_names)) { - current_names <- rep(NA_character_, vcount(graph)) + current_names <- rep_len(NA_character_, vcount(graph)) } idx_numeric <- as.numeric(index) From d52292c1697425e6cbc4d295c2834f19fbde5dd8 Mon Sep 17 00:00:00 2001 From: David Schoch Date: Tue, 15 Jul 2025 15:59:17 +0200 Subject: [PATCH 3/8] Update R/attributes.R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Maëlle Salmon --- R/attributes.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/attributes.R b/R/attributes.R index 62ccabf1b3a..af84e0a02fd 100644 --- a/R/attributes.R +++ b/R/attributes.R @@ -566,7 +566,7 @@ i_set_vertex_attr <- function( igraph_attr_idx_vertex ) - #Uniqueness check for vertex "name" attribute + # Uniqueness check for vertex "name" attribute if (name == "name") { current_names <- vattrs[["name"]] if (is.null(current_names)) { From 0857ede1926a3d30e8b61bfeee14ad2218436892 Mon Sep 17 00:00:00 2001 From: schochastics Date: Tue, 15 Jul 2025 16:02:59 +0200 Subject: [PATCH 4/8] unaffected_names instead of names_unaffected --- R/attributes.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/attributes.R b/R/attributes.R index af84e0a02fd..4f023d4a54e 100644 --- a/R/attributes.R +++ b/R/attributes.R @@ -575,14 +575,14 @@ i_set_vertex_attr <- function( idx_numeric <- as.numeric(index) unaffected <- setdiff(seq_along(current_names), idx_numeric) - names_unaffected <- current_names[unaffected] + unaffected_names <- current_names[unaffected] if (length(value) == 1 && length(idx_numeric) > 1) { value <- rep(value, length(idx_numeric)) } - if (any(value %in% names_unaffected)) { - dupes <- value[value %in% names_unaffected] + if (any(value %in% unaffected_names)) { + dupes <- value[value %in% unaffected_names] cli::cli_abort( "Vertex name{?s} already exist{?s}: {paste(unique(dupes), collapse = ', ')}" ) From 5030459e6095b82dae27b0c47da42beeb7f6cbd1 Mon Sep 17 00:00:00 2001 From: David Schoch Date: Tue, 15 Jul 2025 16:03:31 +0200 Subject: [PATCH 5/8] Update R/attributes.R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Maëlle Salmon --- R/attributes.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/attributes.R b/R/attributes.R index 4f023d4a54e..d6148e52c7e 100644 --- a/R/attributes.R +++ b/R/attributes.R @@ -578,7 +578,7 @@ i_set_vertex_attr <- function( unaffected_names <- current_names[unaffected] if (length(value) == 1 && length(idx_numeric) > 1) { - value <- rep(value, length(idx_numeric)) + value <- rep_len(value, length(idx_numeric)) } if (any(value %in% unaffected_names)) { From d1e072f45c1419feb42536b80efc5f7d8b657586 Mon Sep 17 00:00:00 2001 From: David Schoch Date: Tue, 15 Jul 2025 16:04:58 +0200 Subject: [PATCH 6/8] Update R/interface.R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Maëlle Salmon --- R/interface.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/interface.R b/R/interface.R index 786cde8c555..da03e1b50c0 100644 --- a/R/interface.R +++ b/R/interface.R @@ -213,7 +213,7 @@ add_vertices <- function(graph, nv, ..., attr = list()) { attrs <- append(attrs, attr) nam <- names(attrs) if (length(attrs) != 0 && (is.null(nam) || any(nam == ""))) { - cli::cli_abort("please supply names for attributes") + cli::cli_abort("Attribute names must be supplied.") } vertices.orig <- vcount(graph) From 2097a8ba26ee5d97044b35cdcf6c1b9e55c0e984 Mon Sep 17 00:00:00 2001 From: schochastics Date: Tue, 15 Jul 2025 16:09:16 +0200 Subject: [PATCH 7/8] fixed error message --- R/attributes.R | 3 ++- tests/testthat/_snaps/attributes.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/R/attributes.R b/R/attributes.R index d6148e52c7e..7c5b1665128 100644 --- a/R/attributes.R +++ b/R/attributes.R @@ -584,7 +584,8 @@ i_set_vertex_attr <- function( if (any(value %in% unaffected_names)) { dupes <- value[value %in% unaffected_names] cli::cli_abort( - "Vertex name{?s} already exist{?s}: {paste(unique(dupes), collapse = ', ')}" + "{cli::qty(length(unique(dupes)))} Vertex {?name/names} already {?exists/exist}: + {paste(unique(dupes), collapse = ', ')}" ) } } diff --git a/tests/testthat/_snaps/attributes.md b/tests/testthat/_snaps/attributes.md index 6b19b34baf8..03a82097e27 100644 --- a/tests/testthat/_snaps/attributes.md +++ b/tests/testthat/_snaps/attributes.md @@ -68,5 +68,5 @@ add_vertices(g, nv = 2, attr = list(name = c("A", "B"))) Condition Error in `i_set_vertex_attr()`: - ! Vertex name already exist: A, B + ! Vertex names already exist: A, B From 8f23ffa02a368b43e6e31aad72fe4f54144ed970 Mon Sep 17 00:00:00 2001 From: schochastics Date: Tue, 15 Jul 2025 16:31:21 +0200 Subject: [PATCH 8/8] removed redundant numeric index and added another error message --- R/attributes.R | 9 +++++---- tests/testthat/_snaps/attributes.md | 9 +++++++++ tests/testthat/test-attributes.R | 7 +++++++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/R/attributes.R b/R/attributes.R index 5de12b69da0..7cbcd784c23 100644 --- a/R/attributes.R +++ b/R/attributes.R @@ -622,13 +622,14 @@ i_set_vertex_attr <- function( if (is.null(current_names)) { current_names <- rep_len(NA_character_, vcount(graph)) } - idx_numeric <- as.numeric(index) - unaffected <- setdiff(seq_along(current_names), idx_numeric) + unaffected <- setdiff(seq_along(current_names), index) unaffected_names <- current_names[unaffected] - if (length(value) == 1 && length(idx_numeric) > 1) { - value <- rep_len(value, length(idx_numeric)) + if (length(value) == 1 && length(index) > 1) { + cli::cli_abort( + "Cannot set vertex attribute {.arg {name}} to a single value ({value}) for multiple vertices because it results in duplicate names." + ) } if (any(value %in% unaffected_names)) { diff --git a/tests/testthat/_snaps/attributes.md b/tests/testthat/_snaps/attributes.md index 81569880f51..295eeb44f7f 100644 --- a/tests/testthat/_snaps/attributes.md +++ b/tests/testthat/_snaps/attributes.md @@ -69,6 +69,15 @@ Condition Error in `i_set_vertex_attr()`: ! Vertex names already exist: A, B + +--- + + Code + set_vertex_attr(g, "name", 2:3, "C") + Condition + Error in `i_set_vertex_attr()`: + ! Cannot set vertex attribute `name` to a single value (C) for multiple vertices because it results in duplicate names. + # set_vertex_attrs() works Code diff --git a/tests/testthat/test-attributes.R b/tests/testthat/test-attributes.R index 87f3a6013c7..684dc40ece7 100644 --- a/tests/testthat/test-attributes.R +++ b/tests/testthat/test-attributes.R @@ -476,6 +476,13 @@ test_that("duplicated vertex names are handled correctly", { expect_snapshot(error = TRUE, { add_vertices(g, nv = 2, attr = list(name = c("A", "B"))) }) + + g <- graph_from_literal("A"--"B", "C"--"D") + expect_snapshot(error = TRUE, { + set_vertex_attr(g, "name", 2:3, "C") + }) +}) + test_that("set_vertex_attrs() works", { g <- make_ring(10) g <- set_vertex_attrs(g, color = "blue", size = 10, name = LETTERS[1:10])