Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: test-conversion #1539

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions tests/testthat/_snaps/conversion.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,38 @@
Error in `get.adjacency.dense()`:
! Matrices must be either numeric or logical, and the edge attribute is not

# as_long_data_frame() works correctly with and without names

Code
ring <- make_ring(3)
as_long_data_frame(ring)
Output
from to
1 1 2
2 2 3
3 1 3
Code
V(ring)$name <- letters[1:3]
as_long_data_frame(ring)
Output
from to from_name to_name
1 1 2 a b
2 2 3 b c
3 1 3 a c
Code
V(ring)$score <- LETTERS[1:3]
as_long_data_frame(ring)
Output
from to from_name from_score to_name to_score
1 1 2 a A b B
2 2 3 b B c C
3 1 3 a A c C
Code
E(ring)$info <- 3:1
as_long_data_frame(ring)
Output
from to info from_name from_score to_name to_score
1 1 2 3 a A b B
2 2 3 2 b B c C
3 1 3 1 a A c C

148 changes: 148 additions & 0 deletions tests/testthat/test-conversion.R
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,151 @@ test_that("as_adjacency_matrix() works -- dense + not both", {
)
)
})

test_that("as_edgelist() works", {
g <- sample_gnp(100, 3 / 100)
e <- as_edgelist(g)
g2 <- make_graph(t(e), n = vcount(g), dir = FALSE)
expect_isomorphic(g, g2)
})

test_that("as_adj_list() works", {
g <- sample_gnp(50, 2 / 50)
al <- as_adj_list(g)
g2 <- graph_from_adj_list(al, mode = "all")
expect_isomorphic(g, g2)
expect_true(isomorphic(g, g2,
vertex.color1 = 1:vcount(g),
vertex.color2 = 1:vcount(g2),
method = "vf2"
))

####

el <- as_adj_edge_list(g)
for (i in 1:vcount(g)) {
a <- E(g)[.inc(i)]
expect_equal(length(a), length(el[[i]]), ignore_attr = TRUE)
expect_equal(sort(el[[i]]), sort(a), ignore_attr = TRUE)
}

g <- sample_gnp(50, 4 / 50, directed = TRUE)
el1 <- as_adj_edge_list(g, mode = "out")
el2 <- as_adj_edge_list(g, mode = "in")
for (i in 1:vcount(g)) {
a <- E(g)[.from(i)]
expect_equal(length(a), length(el1[[i]]), ignore_attr = TRUE)
expect_equal(sort(el1[[i]]), sort(a), ignore_attr = TRUE)
}
for (i in 1:vcount(g)) {
a <- E(g)[.to(i)]
expect_equal(length(a), length(el2[[i]]), ignore_attr = TRUE)
expect_equal(sort(el2[[i]]), sort(a), ignore_attr = TRUE)
}
})

test_that("graph_from_graphnel() works", {
skip_if_not_installed("graph")
suppressPackageStartupMessages(library(graph, warn.conflicts = FALSE))

g <- sample_gnp(100, 5 / 100)
N <- as_graphnel(g)
g2 <- graph_from_graphnel(N)
gi <- graph.isomorphic.vf2(g, g2)
expect_true(gi$iso)
expect_equal(gi$map12, 1:vcount(g))
expect_equal(gi$map21, 1:vcount(g))

## Attributes

V(g)$name <- as.character(vcount(g):1)
E(g)$weight <- sample(1:10, ecount(g), replace = TRUE)
g$name <- "Foobar"

N <- as_graphnel(g)
g2 <- graph_from_graphnel(N)
expect_isomorphic(g, g2)
expect_equal(V(g)$name, V(g2)$name)

A <- as_adjacency_matrix(g, attr = "weight", sparse = FALSE)
A2 <- as_adjacency_matrix(g2, attr = "weight", sparse = FALSE)
expect_equal(A, A)
expect_equal(g$name, g2$name)
})

test_that("as_graphnel() does not duplicate loop edges", {
skip_if_not_installed("graph")

mat <- matrix(c(1, 0.5, 0.5, 0), nrow = 2)
dimnames(mat) <- list(c("A", "B"), c("A", "B"))

igr <- graph_from_adjacency_matrix(mat, mode = "undirected", weighted = TRUE)

grNEL <- as_graphnel(igr)
expect_equal(graph::edgeL(grNEL)$A$edges, c(1, 2))
})


test_that("as_long_data_frame() works correctly with and without names", {
expect_snapshot({
ring <- make_ring(3)
as_long_data_frame(ring)

V(ring)$name <- letters[1:3]
as_long_data_frame(ring)

V(ring)$score <- LETTERS[1:3]
as_long_data_frame(ring)

E(ring)$info <- 3:1
as_long_data_frame(ring)
})
})

test_that("as_biadjacency_matrix() works -- dense", {
I <- matrix(sample(0:1, 35, replace = TRUE, prob = c(3, 1)), ncol = 5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpick: Let's not call this I, as that's the standard name for the identity matrix. (It was probably called I because of "incidence" matrix.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and good to merge once I fix that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is good, and this is just an aesthetic nitpick that doesn't necessarily need to be fixed. My main comments are in the other thread in this PR, addressing string matrices. Will open an issue for that soon.

g <- graph_from_biadjacency_matrix(I)
I2 <- as_biadjacency_matrix(g)
expect_equal(I, I2, ignore_attr = TRUE)
expect_identical(rownames(I2), as.character(1:7))
expect_identical(colnames(I2), as.character(8:12))
})

test_that("as_biadjacency_matrix() works -- dense named", {
I <- matrix(sample(0:1, 35, replace = TRUE, prob = c(3, 1)), ncol = 5)
g <- graph_from_biadjacency_matrix(I)
V(g)$name <- letters[1:length(V(g))]

expect_true(is_named(g))

I2 <- as_biadjacency_matrix(g)
expect_equal(I, I2, ignore_attr = TRUE)
expect_identical(rownames(I2), c("a", "b", "c", "d", "e", "f", "g"))
expect_identical(colnames(I2), c("h", "i", "j", "k", "l"))
})

test_that("as_biadjacency_matrix() works -- dense + attribute", {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szhorvat does this test make any sense? It's also needed for #1518

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The C core will not support adjacency matrices that contain other values than numbers. This means that we either need to drop support for character adjacency matrices, or we need to implement this behaviour in pure R while making sure to match the C core.

There are several things that don't make much sense with non-numerical adjacency matrices. For example, what do you do with multi-edges? How do you combine their character properties? For plain adjacency matrices, what do you do with self-loops in undirected graphs (whose value is normally doubled in the matrix)?

I'm sure we had a discussion about this in the past, and even reached out to someone who used character matrices (very few people do). But I don't remember the decision. We need to search the issue tracker.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the string "0" for non-existent edges makes no sense to make. Numbers should be numbers and strings should be strings—R is not Perl! 😆

If we really want to support string matrices, wouldn't "" make a lot more sense?

The question of what is the "null strings" also ties into what w do with self-loops and multi-edges. If the multi-edge combiner operation is string concatenation, then the null string is "".

These complications illustrate why I don't particularly like the idea of supporting string matrices.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest aiming to remove string matrix support, and not producing characterization tests for it. Issue: #1542

withr::local_seed(42)
I <- matrix(sample(0:1, 9, replace = TRUE, prob = c(3, 1)), ncol = 3)
g <- graph_from_biadjacency_matrix(I)
E(g)$something <- letters[1:ecount(g)]

I2 <- as_biadjacency_matrix(g, attr = "something")
expect_equal(
unname(I2),
matrix(
c("a", "c", "0", "b", "0", "0", "0", "0", "0"),
nrow = 3L,
ncol = 3L
)
)
})

test_that("as_biadjacency_matrix() works -- sparse", {
I <- matrix(sample(0:1, 35, replace = TRUE, prob = c(3, 1)), ncol = 5)
g <- graph_from_biadjacency_matrix(I)
I3 <- as_biadjacency_matrix(g, sparse = TRUE)
expect_equal(as.matrix(I3), I, ignore_attr = TRUE)
expect_identical(rownames(I3), as.character(1:7))
expect_identical(colnames(I3), as.character(8:12))
})
33 changes: 0 additions & 33 deletions tests/testthat/test-get.adjlist.R

This file was deleted.

6 changes: 0 additions & 6 deletions tests/testthat/test-get.edgelist.R

This file was deleted.

30 changes: 0 additions & 30 deletions tests/testthat/test-get.incidence.R

This file was deleted.

16 changes: 0 additions & 16 deletions tests/testthat/test-graph.data.frame.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,3 @@ test_that("graph_from_data_frame works on matrices", {
el2 <- as_data_frame(g)
expect_equal(as.data.frame(el), el2, ignore_attr = TRUE)
})

test_that("as_long_data_frame() works correctly with and without names", {
expect_snapshot({
ring <- make_ring(3)
as_long_data_frame(ring)

V(ring)$name <- letters[1:3]
as_long_data_frame(ring)

V(ring)$score <- LETTERS[1:3]
as_long_data_frame(ring)

E(ring)$info <- 3:1
as_long_data_frame(ring)
})
})
41 changes: 0 additions & 41 deletions tests/testthat/test-graphNEL.R

This file was deleted.

Loading