Skip to content

Commit a4aa43d

Browse files
Copilotkrlmlr
andcommitted
fix: use vertex count instead of edge count for empty graph check
- Changed check from ecount(e1) > 0 to vcount(e1) > 0 - Empty graphs are now correctly defined as having 0 vertices, not 0 edges - Updated tests to use make_empty_graph() instead of make_empty_graph(1) - Added tests for make_empty_graph(1) to verify it now errors correctly Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
1 parent 8723eee commit a4aa43d

File tree

3 files changed

+41
-18
lines changed

3 files changed

+41
-18
lines changed

R/operators.R

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,8 +1162,8 @@ path <- function(...) {
11621162
)
11631163

11641164
# Abort if adding named vertices to a non-empty unnamed graph
1165-
# Allow adding to graphs with no edges (like make_empty_graph(n))
1166-
if (!is.null(nn) && !is_named(e1) && ecount(e1) > 0) {
1165+
# Empty graphs have zero vertices
1166+
if (!is.null(nn) && !is_named(e1) && vcount(e1) > 0) {
11671167
cli::cli_abort(
11681168
"Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have {.code NA} names."
11691169
)
@@ -1197,8 +1197,8 @@ path <- function(...) {
11971197
} else if (is.character(e2)) {
11981198
## Adding named vertices
11991199
# Abort if adding named vertices to a non-empty unnamed graph
1200-
# Allow adding to graphs with no edges (like make_empty_graph(n))
1201-
if (!is_named(e1) && ecount(e1) > 0) {
1200+
# Empty graphs have zero vertices
1201+
if (!is_named(e1) && vcount(e1) > 0) {
12021202
cli::cli_abort(
12031203
"Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have {.code NA} names."
12041204
)

tests/testthat/_snaps/operators.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
Can't recycle `name` (size 2) to match `foo` (size 3).
44

5+
---
6+
7+
Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have `NA` names.
8+
59
# vertices() errors on duplicate attribute names
610

711
Duplicate attribute name in `vertices()`: "name".
@@ -34,3 +38,11 @@
3438

3539
Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have `NA` names.
3640

41+
---
42+
43+
Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have `NA` names.
44+
45+
---
46+
47+
Cannot add named vertices to a non-empty unnamed graph. Existing vertices will have `NA` names.
48+

tests/testthat/test-operators.R

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -214,27 +214,28 @@ test_that("t() is aliased to edge reversal for graphs", {
214214
})
215215

216216
test_that("vertices() works", {
217-
g_all_unnamed <- make_empty_graph(1) + vertices("a", "b")
217+
# Test adding named vertices to empty graph (0 vertices)
218+
g_all_unnamed <- make_empty_graph() + vertices("a", "b")
218219
expect_s3_class(V(g_all_unnamed), "igraph.vs")
219-
expect_identical(V(g_all_unnamed)$name, c(NA, "a", "b"))
220+
expect_identical(V(g_all_unnamed)$name, c("a", "b"))
220221

221-
g_mix_named_unnamed <- make_empty_graph(1) + vertices("a", "b", foo = 5)
222+
g_mix_named_unnamed <- make_empty_graph() + vertices("a", "b", foo = 5)
222223
expect_s3_class(V(g_mix_named_unnamed), "igraph.vs")
223-
expect_true(is.na(V(g_mix_named_unnamed)$name[1]))
224-
expect_identical(V(g_mix_named_unnamed)$name[-1], c("a", "b"))
225-
expect_equal(V(g_mix_named_unnamed)$foo, c(NA, 5, 5))
224+
expect_identical(V(g_mix_named_unnamed)$name, c("a", "b"))
225+
expect_equal(V(g_mix_named_unnamed)$foo, c(5, 5))
226226

227-
g_mix_bigger_attribute <- make_empty_graph(1) +
227+
g_mix_bigger_attribute <- make_empty_graph() +
228228
vertices("a", "b", "c", foo = 5:7, bar = 8)
229229
expect_s3_class(V(g_mix_bigger_attribute), "igraph.vs")
230-
expect_identical(V(g_mix_bigger_attribute)$name, c(NA, "a", "b", "c"))
231-
expect_equal(V(g_mix_bigger_attribute)$foo, c(NA, 5, 6, 7))
232-
expect_equal(V(g_mix_bigger_attribute)$bar, c(NA, 8, 8, 8))
230+
expect_identical(V(g_mix_bigger_attribute)$name, c("a", "b", "c"))
231+
expect_equal(V(g_mix_bigger_attribute)$foo, c(5, 6, 7))
232+
expect_equal(V(g_mix_bigger_attribute)$bar, c(8, 8, 8))
233233

234-
g_one_unnamed <- make_empty_graph(1) + vertices(letters)
234+
g_one_unnamed <- make_empty_graph() + vertices(letters)
235235
expect_s3_class(V(g_one_unnamed), "igraph.vs")
236-
expect_identical(V(g_one_unnamed)$name, c(NA, letters))
236+
expect_identical(V(g_one_unnamed)$name, letters)
237237

238+
# Test adding unnamed vertices (attributes only) - should work with any graph
238239
g_all_named <- make_empty_graph(1) + vertices(foo = 5:7)
239240
expect_s3_class(V(g_all_named), "igraph.vs")
240241
expect_null(V(g_all_named)$name)
@@ -249,8 +250,14 @@ test_that("vertices() works", {
249250
expect_s3_class(V(g_none), "igraph.vs")
250251
expect_null(V(g_none)$name)
251252

253+
# Test that adding named vertices to non-empty graph with mismatched attributes errors
252254
expect_snapshot_error(
253-
make_empty_graph(1) + vertices("a", "b", foo = 5:7)
255+
make_empty_graph() + vertices("a", "b", foo = 5:7)
256+
)
257+
258+
# Test that adding named vertices to non-empty unnamed graph errors
259+
expect_snapshot_error(
260+
make_empty_graph(1) + vertices("a", "b")
254261
)
255262
})
256263

@@ -306,10 +313,14 @@ test_that("adding named vertices to non-empty unnamed graphs errors", {
306313
expect_no_error(make_ring(10) + vertices(foo = 1:3))
307314
expect_no_error(make_ring(10) + 5)
308315

309-
# No error when adding to empty graph
316+
# No error when adding to empty graph (0 vertices)
310317
expect_no_error(make_empty_graph() + vertex("a"))
311318
expect_no_error(make_empty_graph() + c("a", "b"))
312319
expect_no_error(make_empty_graph() + vertices("a", "b"))
320+
321+
# Error when adding to non-empty unnamed graph (even with 1 vertex)
322+
expect_snapshot_error(make_empty_graph(1) + vertex("a"))
323+
expect_snapshot_error(make_empty_graph(1) + c("a", "b"))
313324
})
314325

315326

0 commit comments

Comments
 (0)