refactor: abort instead of warn, allow adding to empty graphs

- Changed from cli_warn to cli_abort for non-empty unnamed graphs
- Allow adding named vertices to graphs with no edges (empty graphs)
- Added snapshot tests for error messages
- Removed suppressWarnings from tests that should now work

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

Author: Copilot

R/operators.R

@@ -1161,10 +1161,11 @@ path <- function(...) {
       e2[named]
     )
 
-    # Warn if adding named vertices to an unnamed graph
-    if (!is.null(nn) && !is_named(e1)) {
-      cli::cli_warn(
-        "Adding named vertices to an unnamed graph. Existing vertices will have {.code NA} names."
+    # Abort if adding named vertices to a non-empty unnamed graph
+    # Allow adding to graphs with no edges (like make_empty_graph(n))
+    if (!is.null(nn) && !is_named(e1) && ecount(e1) > 0) {
+      cli::cli_abort(
+        "Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have {.code NA} names."
       )
     }
 
@@ -1195,10 +1196,11 @@ path <- function(...) {
     res <- add_vertices(e1, e2)
   } else if (is.character(e2)) {
     ## Adding named vertices
-    # Warn if adding named vertices to an unnamed graph
-    if (!is_named(e1)) {
-      cli::cli_warn(
-        "Adding named vertices to an unnamed graph. Existing vertices will have {.code NA} names."
+    # Abort if adding named vertices to a non-empty unnamed graph
+    # Allow adding to graphs with no edges (like make_empty_graph(n))
+    if (!is_named(e1) && ecount(e1) > 0) {
+      cli::cli_abort(
+        "Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have {.code NA} names."
       )
     }
     res <- add_vertices(e1, length(e2), name = e2)

tests/testthat/_snaps/operators.md

@@ -18,3 +18,19 @@
 
     Duplicate attribute names in `vertices()`: "foo" and "bar".
 
+# adding named vertices to non-empty unnamed graphs errors
+
+    Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have `NA` names.
+
+---
+
+    Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have `NA` names.
+
+---
+
+    Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have `NA` names.
+
+---
+
+    Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have `NA` names.
+

tests/testthat/test-make.R

@@ -231,21 +231,17 @@ test_that("make_graph works for numeric edges and isolates", {
 
 test_that("make_graph handles names", {
   graph_make_names <- make_graph(letters[1:10])
-  graph_elist_names <- suppressWarnings(
-    make_empty_graph() +
-      vertices(letters[1:10]) +
-      edges(letters[1:10])
-  )
+  graph_elist_names <- make_empty_graph() +
+    vertices(letters[1:10]) +
+    edges(letters[1:10])
   expect_identical_graphs(graph_make_names, graph_elist_names)
 })
 
 test_that("make_graph handles names and isolates", {
   graph_make_iso <- make_graph(letters[1:10], isolates = letters[11:20])
-  graph_elist_iso <- suppressWarnings(
-    make_empty_graph() +
-      vertices(letters[1:20]) +
-      edges(letters[1:10])
-  )
+  graph_elist_iso <- make_empty_graph() +
+    vertices(letters[1:20]) +
+    edges(letters[1:10])
   expect_identical_graphs(graph_make_iso, graph_elist_iso)
 })
 

tests/testthat/test-operators.R

@@ -214,28 +214,24 @@ test_that("t() is aliased to edge reversal for graphs", {
 })
 
 test_that("vertices() works", {
-  g_all_unnamed <- suppressWarnings(make_empty_graph(1) + vertices("a", "b"))
+  g_all_unnamed <- make_empty_graph(1) + vertices("a", "b")
   expect_s3_class(V(g_all_unnamed), "igraph.vs")
   expect_identical(V(g_all_unnamed)$name, c(NA, "a", "b"))
 
-  g_mix_named_unnamed <- suppressWarnings(
-    make_empty_graph(1) + vertices("a", "b", foo = 5)
-  )
+  g_mix_named_unnamed <- make_empty_graph(1) + vertices("a", "b", foo = 5)
   expect_s3_class(V(g_mix_named_unnamed), "igraph.vs")
   expect_true(is.na(V(g_mix_named_unnamed)$name[1]))
   expect_identical(V(g_mix_named_unnamed)$name[-1], c("a", "b"))
   expect_equal(V(g_mix_named_unnamed)$foo, c(NA, 5, 5))
 
-  g_mix_bigger_attribute <- suppressWarnings(
-    make_empty_graph(1) +
-      vertices("a", "b", "c", foo = 5:7, bar = 8)
-  )
+  g_mix_bigger_attribute <- make_empty_graph(1) +
+    vertices("a", "b", "c", foo = 5:7, bar = 8)
   expect_s3_class(V(g_mix_bigger_attribute), "igraph.vs")
   expect_identical(V(g_mix_bigger_attribute)$name, c(NA, "a", "b", "c"))
   expect_equal(V(g_mix_bigger_attribute)$foo, c(NA, 5, 6, 7))
   expect_equal(V(g_mix_bigger_attribute)$bar, c(NA, 8, 8, 8))
 
-  g_one_unnamed <- suppressWarnings(make_empty_graph(1) + vertices(letters))
+  g_one_unnamed <- make_empty_graph(1) + vertices(letters)
   expect_s3_class(V(g_one_unnamed), "igraph.vs")
   expect_identical(V(g_one_unnamed)$name, c(NA, letters))
 
@@ -253,9 +249,9 @@ test_that("vertices() works", {
   expect_s3_class(V(g_none), "igraph.vs")
   expect_null(V(g_none)$name)
 
-  expect_snapshot_error(suppressWarnings(
+  expect_snapshot_error(
     make_empty_graph(1) + vertices("a", "b", foo = 5:7)
-  ))
+  )
 })
 
 test_that("vertices() errors on duplicate attribute names", {
@@ -280,39 +276,40 @@ test_that("vertices() errors on duplicate attribute names", {
   )
 })
 
-test_that("adding named vertices to unnamed graphs warns", {
+test_that("adding named vertices to non-empty unnamed graphs errors", {
   # Test with vertex() function
-  expect_warning(
-    make_ring(10) + vertex(1),
-    "Adding named vertices to an unnamed graph"
+  expect_snapshot_error(
+    make_ring(10) + vertex(1)
   )
 
-  expect_warning(
-    make_ring(10) + vertex("a"),
-    "Adding named vertices to an unnamed graph"
+  expect_snapshot_error(
+    make_ring(10) + vertex("a")
   )
 
-  expect_warning(
-    make_ring(10) + vertices("a", "b"),
-    "Adding named vertices to an unnamed graph"
+  expect_snapshot_error(
+    make_ring(10) + vertices("a", "b")
   )
 
   # Test with character vector
-  expect_warning(
-    make_ring(10) + c("a", "b"),
-    "Adding named vertices to an unnamed graph"
+  expect_snapshot_error(
+    make_ring(10) + c("a", "b")
   )
 
-  # No warning when adding to named graph
+  # No error when adding to named graph
   g <- make_ring(10)
   V(g)$name <- letters[1:10]
-  expect_no_warning(g + vertex("k"))
-  expect_no_warning(g + c("x", "y"))
-
-  # No warning when adding unnamed vertices
-  expect_no_warning(make_ring(10) + vertex(foo = 5))
-  expect_no_warning(make_ring(10) + vertices(foo = 1:3))
-  expect_no_warning(make_ring(10) + 5)
+  expect_no_error(g + vertex("k"))
+  expect_no_error(g + c("x", "y"))
+
+  # No error when adding unnamed vertices
+  expect_no_error(make_ring(10) + vertex(foo = 5))
+  expect_no_error(make_ring(10) + vertices(foo = 1:3))
+  expect_no_error(make_ring(10) + 5)
+
+  # No error when adding to empty graph
+  expect_no_error(make_empty_graph() + vertex("a"))
+  expect_no_error(make_empty_graph() + c("a", "b"))
+  expect_no_error(make_empty_graph() + vertices("a", "b"))
 })
 
 

tests/testthat/test-print.R

@@ -46,7 +46,7 @@ test_that("print.igraph() works", {
   })
   expect_output(print(g6), "     ")
 
-  kite <- suppressWarnings(make_empty_graph(directed = FALSE) + LETTERS[1:10])
+  kite <- make_empty_graph(directed = FALSE) + LETTERS[1:10]
   kite <- kite +
     edges(
       "A", "B",

tests/testthat/test-topology.R

@@ -46,11 +46,9 @@ test_that("automorphism_group works with colored graphs", {
 })
 
 test_that("isomorphisms() works", {
-  motif <- suppressWarnings(
-    make_empty_graph(directed = FALSE) +
-      vertices("D1", "D2", type = c("type1", "type1")) +
-      edges("D1", "D2", type = c("type2"))
-  )
+  motif <- make_empty_graph(directed = FALSE) +
+    vertices("D1", "D2", type = c("type1", "type1")) +
+    edges("D1", "D2", type = c("type2"))
   motif_iso <- isomorphisms(
     motif,
     motif,
@@ -63,11 +61,9 @@ test_that("isomorphisms() works", {
 })
 
 test_that("subgraph_isomorphisms works", {
-  motif <- suppressWarnings(
-    make_empty_graph(directed = FALSE) +
-      vertices("D1", "D2", type = c("type1", "type1")) +
-      edges("D1", "D2", type = c("type2"))
-  )
+  motif <- make_empty_graph(directed = FALSE) +
+    vertices("D1", "D2", type = c("type1", "type1")) +
+    edges("D1", "D2", type = c("type2"))
   out <- subgraph_isomorphisms(
     target = motif,
     pattern = motif,